8000 Removing unneeded get_test_info call by rquidute · Pull Request #39967 · project-chip/connectedhomeip · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Removing unneeded get_test_info call #39967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rquidute
Copy link
Contributor
@rquidute rquidute commented Jul 11, 2025

Summary

This PR removes the call get_test_info(test_class, matter_test_config) since it doesn't seem to be doing anything in this context.

Related issues

project-chip/certification-tool-backend#242

Testing

  • Running the following command successfully:
    python3 /root/python_testing/scripts/sdk/TC_CADMIN_1_11.py --commissioning-method on-network --trace-to json:log --discriminator 3840 --passcode 20202021

  • Running using TH client successfully (Commissioning):
    python3 /root/python_testing/scripts/sdk/matter_testing_infrastructure/chip/testing/test_harness_client.py commission --trace-to json:log --commissioning-method on-network --discriminator 3840 --passcode 20202021 --storage_path /root/python_testing/admin_storage2.json

  • Running using TH client successfully (Test Execution):
    python3 /root/python_testing/scripts/sdk/matter_testing_infrastructure/chip/testing/test_harness_client.py sdk/TC_ACE_1_3 TC_ACE_1_3 --tests test_TC_ACE_1_3 --trace-to json:log --in-test-commissioning-method on-network --discriminator 3840 --passcode 20202021

@github-actions github-actions bot added the tests label Jul 11, 2025
Copy link
Contributor
@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request prevents an error by skipping a call to get_test_info for CommissionDeviceTest. The feedback suggests using is not for comparing class types.

…runner.py

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Copy link
github-actions bot commented Jul 11, 2025

PR #39967: Size comparison from 6d5aa8c to 51fdb21

Full report (59 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen) < 8000 td align="right">6935131
platform target config section 6d5aa8c 51fdb21 change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1102622 1102622 0 0.0
RAM 179010 179010 0 0.0
bl702 lighting-app bl702+eth FLASH 656030 656030 0 0.0
RAM 134961 134961 0 0.0
bl702+wifi FLASH 833212 833212 0 0.0
RAM 124517 124517 0 0.0
bl706+mfd+rpc+littlefs FLASH 1065330 1065330 0 0.0
RAM 117373 117373 0 0.0
bl702l contact-sensor-app bl702l+mfd+littlefs FLASH 894876 894876 0 0.0
RAM 105660 105660 0 0.0
lighting-app bl702l+mfd+littlefs FLASH 978594 978594 0 0.0
RAM 109852 109852 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 763128 763128 0 0.0
RAM 103368 103368 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 774668 774668 0 0.0
RAM 108536 108536 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 721008 721008 0 0.0
RAM 96940 96940 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 705300 705300 0 0.0
RAM 97148 97148 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 548850 548850 0 0.0
RAM 205144 205144 0 0.0
lock CC3235SF_LAUNCHXL FLASH 581842 581842 0 0.0
RAM 205344 205344 0 0.0
efr32 lock-app BRD4187C FLASH 955016 955016 0 0.0
RAM 126564 126564 0 0.0
BRD4338a FLASH 749468 749460 -8 -0.0
RAM 251912 251912 0 0.0
window-app BRD4187C FLASH 1049576 1049576 0 0.0
RAM 122760 122760 0 0.0
esp32 all-clusters-app c3devkit DRAM 102272 102272 0 0.0
FLASH 1780616 1780616 0 0.0
IRAM 83862 83862 0 0.0
m5stack DRAM 121156 121156 0 0.0
FLASH 1747894 1747894 0 0.0
IRAM 117071 117071 0 0.0
linux air-purifier-app debug unknown 4856 4856 0 0.0
FLASH 2796646 2796646 0 0.0
RAM 117320 117320 0 0.0
all-clusters-app debug unknown 5672 5672 0 0.0
FLASH 6198206 6198206 0 0.0
RAM 531216 531216 0 0.0
all-clusters-minimal-app debug unknown 5536 5536 0 0.0
FLASH 5473562 5473562 0 0.0
RAM 228008 228008 0 0.0
bridge-app debug unknown 5568 5568 0 0.0
FLASH 4807802 4807802 0 0.0
RAM 207712 207712 0 0.0
camera-app debug unknown 8976 8976 0 0.0
FLASH 6935131 0 0.0
RAM 230024 230024 0 0.0
camera-controller debug unknown 9216 9216 0 0.0
FLASH 14387339 14387339 0 0.0
RAM 661528 661528 0 0.0
chip-tool debug unknown 6272 6272 0 0.0
FLASH 14738305 14738305 0 0.0
RAM 655072 655072 0 0.0
chip-tool-ipv6only arm64 unknown 40672 40672 0 0.0
FLASH 12713487 12713487 0 0.0
RAM 701344 701344 0 0.0
closure-app debug unknown 5536 5536 0 0.0
FLASH 4790656 4790656 0 0.0
RAM 200584 200584 0 0.0
fabric-admin debug unknown 5952 5952 0 0.0
FLASH 12798837 12798837 0 0.0
RAM 654104 654104 0 0.0
fabric-bridge-app debug unknown 4816 4816 0 0.0
FLASH 4593134 4593134 0 0.0
RAM 193424 193424 0 0.0
fabric-sync debug unknown 5056 5056 0 0.0
FLASH 5741661 5741661 0 0.0
RAM 491728 491728 0 0.0
lighting-app debug+rpc+ui unknown 6280 6280 0 0.0
FLASH 5694593 5694593 0 0.0
RAM 209944 209944 0 0.0
lock-app debug unknown 5488 5488 0 0.0
FLASH 4836482 4836482 0 0.0
RAM 197192 197192 0 0.0
ota-provider-app debug unknown 4856 4856 0 0.0
FLASH 4446986 4446986 0 0.0
RAM 186112 186112 0 0.0
ota-requestor-app debug unknown 4736 4736 0 0.0
FLASH 4519108 4519108 0 0.0
RAM 188984 188984 0 0.0
shell debug unknown 4288 4288 0 0.0
FLASH 3076572 3076572 0 0.0
RAM 147344 147344 0 0.0
thermostat-no-ble arm64 unknown 9832 9832 0 0.0
FLASH 4236319 4236319 0 0.0
RAM 233304 233304 0 0.0
tv-app debug unknown 5824 5824 0 0.0
FLASH 6106653 6106653 0 0.0
RAM 615976 615976 0 0.0
tv-casting-app debug unknown 5352 5352 0 0.0
FLASH 12888029 12888029 0 0.0
RAM 771728 771728 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 888100 888100 0 0.0
RAM 166162 166162 0 0.0
nrf7002dk_nrf5340_cpuapp FLASH 897252 897252 0 0.0
RAM 145100 145100 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 858424 858424 0 0.0
RAM 141049 141049 0 0.0
nxp contact mcxw71+release FLASH 624800 624800 0 0.0
RAM 63164 63164 0 0.0
lock mcxw71+release FLASH 776008 776008 0 0.0
RAM 67820 67820 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1632532 1632532 0 0.0
RAM 211104 211104 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1576708 1576708 0 0.0
RAM 208472 208472 0 0.0
light cy8ckit_062s2_43012 FLASH 1449500 1449500 0 0.0
RAM 197184 197184 0 0.0
lock cy8ckit_062s2_43012 FLASH 1481756 1481756 0 0.0
RAM 224904 224904 0 0.0
qpg lighting-app qpg6200+debug FLASH 744232 744232 0 0.0
RAM 94292 94292 0 0.0
lock-app qpg6200+debug FLASH 753852 753852 0 0.0
RAM 94320 94320 0 0.0
stm32 light STM32WB5MM-DK FLASH 465292 465292 0 0.0
RAM 141376 141376 0 0.0
telink bridge-app tl7218x FLASH 702340 702340 0 0.0
RAM 93600 93600 0 0.0
light-app-ota-compress-lzma-shell-factory-data tl3218x FLASH 794072 794072 0 0.0
RAM 44016 44016 0 0.0
light-app-ota-shell-factory-data tl7218x FLASH 782478 782478 0 0.0
RAM 100912 100912 0 0.0
light-switch-app-ota-compress-lzma-factory-data tl7218x_retention FLASH 709590 709590 0 0.0
RAM 54240 54240 0 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 746184 746184 0 0.0
RAM 77404 77404 0 0.0
light-switch-app-ota-factory-data tl3218x_retention FLASH 722910 722910 0 0.0
RAM 36996 36996 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 603014 603014 0 0.0
RAM 112532 112532 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 818032 818036 4 0.0
RAM 99164 99164 0 0.0
tizen all-clusters-app arm unknown 5096 5096 0 0.0
FLASH 1695816 1695816 0 0.0
RAM 91444 91444 0 0.0
chip-tool-ubsan arm unknown 20764 20764 0 0.0
FLASH 21066690 21066690 0 0.0
RAM 9165440 9165440 0 0.0

@@ -328,7 +328,11 @@ def run_tests_no_exit(
CommissionDeviceTest.event_loop = event_loop
test_class.event_loop = event_loop

get_test_info(test_class, matter_test_config)
# If the test class is not CommissionDeviceTest, retrieve its information.
# CommissionDeviceTest does not implement the methods expected by get_test_info
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you explain why the clas does not implement the methods expected? Why is the fix to add a exception here (which is more tight coupling) rather than adding required methods to the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a very good point and a great suggestion you've raised regarding the method implementation and coupling. Thanks for bringing it up.
Upon further investigation, I've actually uncovered an underlying issue. The get_test_info call, when processing a CommissionDeviceTest class, currently throws an error during the __init__ method of CommissionDeviceTest itself. This seems to be a side effect of a recent refactoring within that class, specifically from PR #39478.

Given that the current behavior of CommissionDeviceTest is causing this __init__ error due to the recent changes, I believe the most robust solution will require collaboration with the author of that original PR.

@jtrejoespinoza-grid Could you help me out with this, please?

Copy link
Contributor Author
@rquidute rquidute Jul 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andy31415 I don't have full context here, but besides adding the required tests, we also need to ensure there are values for default_controller and matter_test_config keys in those two assignments inside CommissionDeviceTest's __init__ method.

if "default_controller" in test_config.user_params:
            self.default_controller = test_config.user_params['default_controller']
if "matter_test_config" in test_config.user_params:
            self.setup_payloads: List[SetupPayloadInfo] = get_setup_payload_info_config(
                global_stash.unstash_globally(test_config.user_params['matter_test_config']))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably this is because before moving commissioning.py into its own file the class CommissionDeviceTest was implemented in matter_testing.py and this class used to extend from MatterBaseTest. Currently on its own file it extends from mobly.base_test.BaseTestClass this affects because it does not have all the methods implemented in the class MatterBaseTest. This was done this way to avoid circular import. Probably some implementation on the class CommissionDeviceTest should be done to clear this issues.

@rquidute
Copy link
Contributor Author
rquidute commented Jul 11, 2025

Hi @andy31415 and @jtrejoespinoza-grid Looking closer, I noticed the get_test_info(test_class, matter_test_config) call doesn't seem to be doing anything in this context.
It returns list[TestInfo], but its return value isn't being used, and the arguments test_class and matter_test_config aren't being modified.

I tested removing it, and the code still behaves as expected.

Do you see any issues with just removing this call?

@andy31415
Copy link
Contributor
andy31415 commented Jul 11, 2025

Hi @andy31415 and @jtrejoespinoza-grid Looking closer, I noticed the get_test_info(test_class, matter_test_config) call doesn't seem to be doing anything in this context. It returns list[TestInfo], but its return value isn't being used, and the arguments test_class and matter_test_config aren't being modified.

I tested removing it, and the code still behaves as expected.

Do you see any issues with just removing this call?

Not sufficiently familiar with this, but if you checked and our tests pass, we can remove

@rquidute
Copy link
Contributor Author

Hi @andy31415 and @jtrejoespinoza-grid Looking closer, I noticed the get_test_info(test_class, matter_test_config) call doesn't seem to be doing anything in this context. It returns list[TestInfo], but its return value isn't being used, and the arguments test_class and matter_test_config aren't being modified.
I tested removing it, and the code still behaves as expected.
Do you see any issues with just removing this call?

Not sufficiently familiar with this, but if you checked and our tests pass, we can remove

I'll wait @jtrejoespinoza-grid 's feedback before proceeding with the change.

@jtrejoespinoza-grid
Copy link
Contributor

Hi @rquidute regarding your comments I made some test by removing the line with the code : get_test_info(test_class, matter_test_config) at runner.py.

I re-built the python files again and the run some test with commissioning on-network with the Linux App and then commissioning ble-wifi with a M5 Stack flashed with LIGTHING_APP and I did not face any commissioning issues.

Let me provide the information that I was able to gather.

Screenshot 2025-07-11 at 3 03 07 p m

Then build python env:

${MATTER_BASE_PATH}/scripts/build_python.sh -i ${MATTER_BASE_PATH}/out/python_env

Then load the env:

source out/python_env/bin/activate

Run one test locally using commissioning on-network using the Linux app:

python3 src/python_testing/TC_CC_2_1.py --commissioning-method on-network --qr-code MT:-24J0AFN00KA0648G00 --endpoint 1

Screenshot 2025-07-11 at 3 09 13 p m

Logs:
CC_2_1.log

Run one test using a device with commissioning ble-wifi:

python3 src/python_testing/TC_CC_2_1.py --commissioning-method ble-wifi --qr-code MT:-24J0AFN00KA0648G00 --wifi-ssid <wifi-ssid> --wifi-passphrase <wifipwd>

Screenshot 2025-07-11 at 3 05 57 p m

Logs :
blewifi.log

@rquidute
Copy link
Contributor Author

Thanks @jtrejoespinoza-grid

Copy link
github-actions bot commented Jul 11, 2025

PR #39967: Size comparison from 6d5aa8c to 336c36a

Full report (59 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
platform target config section 6d5aa8c 336c36a change % change
bl602 lighting-app bl602+mfd+littlefs+rpc FLASH 1102622 1102628 6 0.0
RAM 179010 179010 0 0.0
bl702 lighting-app bl702+eth FLASH 656030 656030 0 0.0
RAM 134961 134961 0 0.0
bl702+wifi FLASH 833212 833218 6 0.0
RAM 124517 124517 0 0.0
bl706+mfd+rpc+littlefs FLASH 1065330 1065336 6 0.0
RAM 117373 117373 0 0.0
bl702l contact-sensor-app bl702l+mfd+littlefs FLASH 894876 894882 6 0.0
RAM 105660 105660 0 0.0
lighting-app bl702l+mfd+littlefs FLASH 978594 978600 6 0.0
RAM 109852 109852 0 0.0
cc13x4_26x4 lighting-app LP_EM_CC1354P10_6 FLASH 763128 763128 0 0.0
RAM 103368 103368 0 0.0
lock-ftd LP_EM_CC1354P10_6 FLASH 774668 774668 0 0.0
RAM 108536 108536 0 0.0
pump-app LP_EM_CC1354P10_6 FLASH 72100 67E6 8 721008 0 0.0
RAM 96940 96940 0 0.0
pump-controller-app LP_EM_CC1354P10_6 FLASH 705300 705300 0 0.0
RAM 97148 97148 0 0.0
cc32xx air-purifier CC3235SF_LAUNCHXL FLASH 548850 548866 16 0.0
RAM 205144 205144 0 0.0
lock CC3235SF_LAUNCHXL FLASH 581842 581842 0 0.0
RAM 205344 205344 0 0.0
efr32 lock-app BRD4187C FLASH 955016 955016 0 0.0
RAM 126564 126564 0 0.0
BRD4338a FLASH 749468 749460 -8 -0.0
RAM 251912 251912 0 0.0
window-app BRD4187C FLASH 1049576 1049576 0 0.0
RAM 122760 122760 0 0.0
esp32 all-clusters-app c3devkit DRAM 102272 102272 0 0.0
FLASH 1780616 1780626 10 0.0
IRAM 83862 83862 0 0.0
m5stack DRAM 121156 121156 0 0.0
FLASH 1747894 1747922 28 0.0
IRAM 117071 117071 0 0.0
linux air-purifier-app debug unknown 4856 4856 0 0.0
FLASH 2796646 2796680 34 0.0
RAM 117320 117320 0 0.0
all-clusters-app debug unknown 5672 5672 0 0.0
FLASH 6198206 6198718 512 0.0
RAM 531216 532176 960 0.2
all-clusters-minimal-app debug unknown 5536 5536 0 0.0
FLASH 5473562 5473624 62 0.0
RAM 228008 228008 0 0.0
bridge-app debug unknown 5568 5568 0 0.0
FLASH 4807802 4807836 34 0.0
RAM 207712 207712 0 0.0
camera-app debug unknown 8976 8976 0 0.0
FLASH 6935131 6945803 10672 0.2
RAM 230024 230168 144 0.1
camera-controller debug unknown 9216 9216 0 0.0
FLASH 14387339 14389035 1696 0.0
RAM 661528 662488 960 0.1
chip-tool debug unknown 6272 6272 0 0.0
FLASH 14738305 14763429 25124 0.2
RAM 655072 656192 1120 0.2
chip-tool-ipv6only arm64 unknown 40672 40736 64 0.2
FLASH 12713487 12733351 19864 0.2
RAM 701344 702440 1096 0.2
closure-app debug unknown 5536 5536 0 0.0
FLASH 4790656 4790690 34 0.0
RAM 200584 200584 0 0.0
fabric-admin debug unknown 5952 5952 0 0.0
FLASH 12798837 12806249 7412 0.1
RAM 654104 655224 1120 0.2
fabric-bridge-app debug unknown 4816 4816 0 0.0
FLASH 4593134 4593166 32 0.0
RAM 193424 193424 0 0.0
fabric-sync debug unknown 5056 5056 0 0.0
FLASH 5741661 5741725 64 0.0
RAM 491728 492688 960 0.2
lighting-app debug+rpc+ui unknown 6280 6280 0 0.0
FLASH 5694593 5694625 32 0.0
RAM 209944 209944 0 0.0
lock-app debug unknown 5488 5488 0 0.0
FLASH 4836482 4836546 64 0.0
RAM 197192 197192 0 0.0
ota-provider-app debug unknown 4856 4856 0 0.0
FLASH 4446986 4447020 34 0.0
RAM 186112 186112 0 0.0
ota-requestor-app debug unknown 4736 4736 0 0.0
FLASH 4519108 4519142 34 0.0
RAM 188984 188984 0 0.0
shell debug unknown 4288 4288 0 0.0
FLASH 3076572 3076636 64 0.0
RAM 147344 147344 0 0.0
thermostat-no-ble arm64 unknown 9832 9832 0 0.0
FLASH 4236319 4236319 0 0.0
RAM 233304 233304 0 0.0
tv-app debug unknown 5824 5824 0 0.0
FLASH 6106653 6106749 96 0.0
RAM 615976 616936 960 0.2
tv-casting-app debug unknown 5352 5352 0 0.0
FLASH 12888029 12907789 19760 0.2
RAM 771728 772688 960 0.1
nrfconnect all-clusters-app nrf52840dk_nrf52840 FLASH 888100 888116 16 0.0
RAM 166162 166162 0 0.0
nrf7002dk_nrf5340_cpuapp FLASH 897252 897252 0 0.0
RAM 145100 145100 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 FLASH 858424 858440 16 0.0
RAM 141049 141049 0 0.0
nxp contact mcxw71+release FLASH 624800 624800 0 0.0
RAM 63164 63164 0 0.0
lock mcxw71+release FLASH 776008 776008 0 0.0
RAM 67820 67820 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 FLASH 1632532 1632532 0 0.0
RAM 211104 211104 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 FLASH 1576708 1576708 0 0.0
RAM 208472 208472 0 0.0
light cy8ckit_062s2_43012 FLASH 1449500 1449500 0 0.0
RAM 197184 197184 0 0.0
lock cy8ckit_062s2_43012 FLASH 1481756 1481756 0 0.0
RAM 224904 224904 0 0.0
qpg lighting-app qpg6200+debug FLASH 744232 744232 0 0.0
RAM 94292 94292 0 0.0
lock-app qpg6200+debug FLASH 7538 A3E2 52 753852 0 0.0
RAM 94320 94320 0 0.0
stm32 light STM32WB5MM-DK FLASH 465292 465292 0 0.0
RAM 141376 141376 0 0.0
telink bridge-app tl7218x FLASH 702340 702350 10 0.0
RAM 93600 93600 0 0.0
light-app-ota-compress-lzma-shell-factory-data tl3218x FLASH 794072 794078 6 0.0
RAM 44016 44016 0 0.0
light-app-ota-shell-factory-data tl7218x FLASH 782478 782484 6 0.0
RAM 100912 100912 0 0.0
light-switch-app-ota-compress-lzma-factory-data tl7218x_retention FLASH 709590 709600 10 0.0
RAM 54240 54240 0 0.0
light-switch-app-ota-compress-lzma-shell-factory-data tlsr9528a FLASH 746184 746194 10 0.0
RAM 77404 77404 0 0.0
light-switch-app-ota-factory-data tl3218x_retention FLASH 722910 722920 10 0.0
RAM 36996 36996 0 0.0
lighting-app-ota-factory-data tlsr9118bdk40d FLASH 603014 603020 6 0.0
RAM 112532 112532 0 0.0
lighting-app-ota-rpc-factory-data-4mb tlsr9518adk80d FLASH 818032 818042 10 0.0
RAM 99164 99164 0 0.0
tizen all-clusters-app arm unknown 5096 5096 0 0.0
FLASH 1695816 1695816 0 0.0
RAM 91444 91444 0 0.0
chip-tool-ubsan arm unknown 20764 20800 36 0.2
FLASH 21066690 21098610 31920 0.2
RAM 9165440 9178492 13052 0.1

@rquidute rquidute changed the title Prevent errors from CommissionDeviceTest's get_test_info calls Removing unneeded get_test_info call Jul 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0