TC-FAN-3.1 - Update: Enhance attribute value testing #36971
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
|
Review changes with |
github-actions
bot
added
tests
matter-1.4-te2-script-change
|
PR #36971: Size comparison from f8d457a to 6c96a7d Full report (54 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #36971: Size comparison from f8d457a to 62f7df7 Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #36971: Size comparison from f8d457a to 643c405 Increases above 0.2%:
Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
andy31415
reviewed
Jan 9, 2025
andy31415
reviewed
Jan 9, 2025
andy31415
reviewed
Jan 9, 2025
andy31415
reviewed
Jan 9, 2025
…eip into TC-FAN-3.1
|
PR #36971: Size comparison from 28c1d83 to 061339c Full report (14 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, stm32, tizen)
|
|
PR #36971: Size comparison from 28c1d83 to 8c2911c Full report (74 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #36971: Size comparison from 28c1d83 to d6c4aac Full report (74 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #36971: Size comparison from 28c1d83 to 61d74b7 Full report (74 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
cecille
reviewed
Mar 4, 2025
There was a problem hiding this comment.
This code needs some restructuring to be readable. You have functions here with really large numbers of input arguments, and functions that appear to be doing two different things based on the input arguments, but checking them at every iteration.
This is just generally really hard to follow because things are grouped together in strange ways.
src/python_testing/TC_FAN_3_1.py
Outdated
| asserts.assert_equal(attr_to_verify_value_current, init_setting, | ||
| f"[FC] Current {attr_to_verify.__name__} attribute value must be equal to the previous value") | ||
| elif order == OrderEnum.Descending: | ||
| asserts.assert_greater(attr_to_verify_value_current, init_setting, |
There was a problem hiding this comment.
something is going weird if you get here, right? how would you have an initial fan mode of off, be adjusting the percent, but be checking in descending order?
There was a problem hiding this comment.
By the time we get here, in the case you're pointing out, which is updating percent descending when initial state is off:
asserts.assert_greater(attr_to_verify_value_current, init_setting,
init_settingis 0, it's the value read before testing/writing begins, in iteration 1 the previous-value takes theinit_settingvalue.attr_to_verify_value_currentis the value after the first write operation in iteration 1, since it's descending, it began at 100.
Therefore the current value (100) must be greater than previous value (initial value) 0.
| # if attr_to_verify_value_current == attr_to_verify_value_previous: | ||
| # attr_to_verify_value_current = None | ||
|
|
||
| if attr_to_verify_value_current is not None: |
There was a problem hiding this comment.
You should never be getting none here because you're never setting the fan mode to auto
There was a problem hiding this comment.
This is the change detection mechanism. As discussed previously, I'm not using expectation logic for this due to the need to also handle the INVALID_IN_STATE status code.
I cannot know beforehand which write operation will result in an INVALID_IN_STATE or SUCCESS status code; therefore, I cannot prepare a list of expected values.
The way it's currently written allows me to check in real time which verification to perform without expectations. A SUCCESS status code means a greater-than or less-than verification will performed, while an INVALID_IN_STATE status code means an equality verification will be performed.
This is unrelated to Auto mode.
src/python_testing/TC_FAN_3_1.py
Outdated
| attr_to_update (str): The attribute that was updated. | ||
| attr_to_verify (str): The attribute whose value needs to be verified. | ||
| order (str): The order in which the attributes are processed. | ||
| init_fan_mode (int): The initial fan mode setting. |
There was a problem hiding this comment.
are these init values JUST passed in for the purposes of logging? if they're not changing, do they need to be passed in every iteration?
There was a problem hiding this comment.
It's not logging every iteration, it's logging the failed assertion message on any one attribute.
More than anything it's meant to provide sufficient detail on what went wrong so the user can quickly catch the error and provide a solution.
This function is run only when the write operation status code is INVALID_IN_STATE, which in general is the exception, so it's not triggered on every iteration from 0 to 100.
…eip into TC-FAN-3.1
|
PR #36971: Size comparison from 36a1bbd to 2c07437 Full report (75 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
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.
Testing
Addresses TC-FAN-3.1 from
[TC-FAN] Fan tests do not sufficiently test attribute interactions #4788
Testing criteria
The main testing being performed is verifying that the values for PercentSetting, FanMode, or SpeedSetting (if supported) are being updated correctly.
Specifically, it checks whether updating PercentSetting or FanMode, which triggers corresponding changes in themselves and the other, results in the current value being greater than or less than the previous value (depending on whether the test is performed in ascending or descending order, respectively). This check is done when the status code for the update is SUCCESS.
Alternatively, when the status code is INVALID_IN_STATE, the test verifies that the current value is the same as the previous value.
This also requires a corresponding test plan update