TC-IDM-4.2 Step 8 delay adjust #31773
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
PR #31773: Size comparison from 30a7636 to 6338e61 Full report (39 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32)
|
pullapprove
bot
requested review from
andy31415,
andyg-apple,
anush-apple,
arkq,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
harsha-rajendran,
hawk248,
hicklin,
jepenven-silabs,
jmartinez-silabs,
jmeg-sfy,
joonhaengHeo,
jtung-apple,
kkasperczyk-no,
kpschoedel,
ksperling-apple and
lazarkov
|
PR #31773: Size comparison from 30a7636 to 6472b8c Decreases (1 build for efr32)
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
tcarmelveilleux
requested changes
Jan 31, 2024
|
PR #31773: Size comparison from 058f199 to 3f26dc8 Decreases (1 build for efr32)
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
|
PR #31773: Size comparison from 573511d to 4c5df4c Decreases (2 builds for efr32)
Full report (45 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32)
|
|
PR #31773: Size comparison from f7494fa to 53c1002 Decreases (2 builds for efr32)
Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
|
Basically the test in the CI was timing out due to performing a read before the value was available due to the different response times in different network environments. The last update implements a loop that times out in 10 seconds that checks for the value in increments of 0.1 seconds until it receives the value. This is a temporary patch until @cecille works out the proper solution. |
|
PR #31773: Size comparison from 3edfb9c to 1d09862 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
cecille
reviewed
Feb 23, 2024
|
|
||
| # Check if the value is not an empty string | ||
| if attribute_value != "": | ||
| loop = False # Exit the loop |
There was a problem hiding this comment.
Suggested change
| loop = False # Exit the loop | |
| return attribute_value |
Then you can get rid of the loop var and just loop on time.time() < end_time and throw the error at the end
cecille
added a commit
to cecille/connectedhomeip
that referenced
this pull request
Feb 29, 2024
Disabled until project-chip#31773 is in or the test is updated to the latest test plan changes.
andy31415
reviewed
Mar 4, 2024
| def get_attribute_value_wait(self, sub, typed_attr_path): | ||
| start_time = time.time() | ||
| timeout = 10 | ||
| increment = 0.1 |
There was a problem hiding this comment.
I think we may have a few of these sprinkled in the code. How about adding a retry library in the testing requirements like https://tenacity.readthedocs.io/en/latest/ or maybe there are others, so we can read code like @retry(stop=stop_after_delay(10), wait=wait_fixed(0.1), retry=retry_if_result(is_empty))) or something like that.
There was a problem hiding this comment.
ugh...I hope we don't. If we do, that seems like a test red flag.
There was a problem hiding this comment.
I put up a PR to disable this test in the CI. The test plan has been updated so we don't have to do this. I'm hoping we can just skip this retry at this point and fix the test properly. This was meant to be a quick fix-forward to get this to stop flaking.
mergify bot
pushed a commit
that referenced
this pull request
Mar 4, 2024
Disabled until #31773 is in or the test is updated to the latest test plan changes.
|
PR #31773: Size comparison from b742587 to f4fa9dd Decreases (1 build for efr32)
Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, k32w, linux, mbed, nrfconnect, psoc6, qpg, stm32, telink)
|
erwinpan1
pushed a commit
to erwinpan1/connectedhomeip
that referenced
this pull request
Mar 7, 2024
Disabled until project-chip#31773 is in or the test is updated to the latest test plan changes.
huangxuyong
pushed a commit
to huangxuyong/connectedhomeip
that referenced
this pull request
Mar 19, 2024
Disabled until project-chip#31773 is in or the test is updated to the latest test plan changes.
cecille
added a commit
to cecille/connectedhomeip
that referenced
this pull request
Apr 12, 2024
Disabled until project-chip#31773 is in or the test is updated to the latest test plan changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds 1/2 second to existing delay in step 8 before reading updated attribute value
From:
#31113