Test runtime and complexity is growing

[AndreRicardo-Zoetis]

This is more of a slight rant/feedback given the difficulties we had in particular with the tests integrating 1.3.0 release. 😆

Expected Behavior

Tests used to run in a few seconds (Total Test time (real) = 6.11 sec) in earlier versions.

Current Behavior

Latest builds take (Total Test time (real) = 22.97 sec) to run.

Steps to Reproduce

Provide a detailed set of steps to reproduce the bug.

1. scripts/build.sh -c -u
2. cd out; ctest

Device Information

• Host OS [e.g. Ubuntu 22.04]: Debian 12
• Architecture [e.g. amd64, arm32, arm64]: amd64
• Provisioning Method:
• DU Agent Version (run 'sudo -u adu /usr/bin/AducIotAgent -v'):
  1.3.0, 1.4.0

Logs

Log was too large but you can see the runtime in Microsoft runs, for example here:

https://github.com/Azure/iot-hub-device-update/actions/runs/28186588000/job/83489957762

Additional Information

As far as we are concerned please do not add generated AI tests to increase coverage just for the sake of it, if those tests test no functionality.
We care for correctness and speed, not coverage metric.

Another annoyance is that every time we integrate a Microsoft release, we have to fix these tests to work with our fork.

[AndreRicardo-Zoetis]

Examples of tests failing in our side and taking a long time to do so:

241/722 Test #241: Download_curl returns failure when entity DownloadUri is empty string ..........................   Passed    0.00 sec
        Start 242: Download_curl returns failure when HashCount is zero, no callback
242/722 Test #242: Download_curl returns failure when HashCount is zero, no callback ..............................***Failed    0.17 sec
        Start 243: Download_curl calls progress callback on HashCount-zero error
243/722 Test #243: Download_curl calls progress callback on HashCount-zero error ..................................***Failed   31.34 sec
        Start 244: Download_curl returns failure with unsupported hash type, no callback
244/722 Test #244: Download_curl returns failure with unsupported hash type, no callback ..........................***Failed   31.34 sec
        Start 245: Download_curl calls progress callback on unsupported hash type
245/722 Test #245: Download_curl calls progress callback on unsupported hash type .................................***Failed    0.09 sec
        Start 246: Download_curl skips download when file already exists with valid hash
246/722 Test #246: Download_curl skips download when file already exists with valid hash ..........................***Failed   31.26 sec
        Start 247: Download_curl skip-download reports progress on success callback
247/722 Test #247: Download_curl skip-download reports progress on success callback ...............................***Failed   31.25 sec
        Start 248: Download_curl returns external-failure when curl command fails
248/722 Test #248: Download_curl returns external-failure when curl command fails .................................   Passed   31.04 sec
        Start 249: Download_curl reports error progress when curl command fails
249/722 Test #249: Download_curl reports error progress when curl command fails ...................................   Passed   31.04 sec
        Start 250: Download_curl succeeds for local file URI with valid hash
250/722 Test #250: Download_curl succeeds for local file URI with valid hash ......................................   Passed    0.01 sec
        Start 251: Download_curl fails after download when hash does not match
251/722 Test #251: Download_curl fails after download when hash does not match ....................................***Failed    0.01 sec

(...)
546/722 Test #546: ADUC_ConfigInfo_Init Functional Tests ..........................................................***Exception: SegFault  0.00 sec

[AndreRicardo-Zoetis]

Example of tests that cover the same behaviour:

1. Both test the same:

   SECTION("Install failure auto-transitions to Restore")

   SECTION("Install failure triggers auto-transition to Restore step")

2. The tests below cover the same as the last comprehensive test:

    SECTION("InstallSucceeded state should NOT report to cloud")
    SECTION("ApplyStarted state should NOT report to cloud")
    SECTION("BackupStarted state should NOT report to cloud")
    SECTION("RestoreStarted state should NOT report to cloud")
(...)

Then there is a for loop test that checks almost exactly the same as above.

TEST_CASE("AgentOrchestration_ShouldNotReportToCloud comprehensive")

The function already encapsulates the behaviour on a simple set of conditions, if the user (such as us) tries to use a different reporting combination, then all the tests break.

[AndreRicardo-Zoetis]

Another example of a set of tests that do not really add much:

    SECTION("SendCommand returns false for command over max length")
    {
        std::string tooLong(80, 'A');
        CHECK(SendCommand(tooLong.c_str()) == false);
    }

Changing this test to a value less than max 64, for example 8 and checking for the result to be true, still fails.

    SECTION("SendCommand returns true for command below max length")
    {
        std::string notTooLong(8, 'A');
        CHECK(SendCommand(notTooLong.c_str()) == true);
    }

The test is checking the result not of the validation, but something else like user permissions and such. In our case these tests are failing because we run them with permissions that actually reach fd = open(ADUC_COMMANDS_FIFO_NAME, O_WRONLY); where it just hangs.

Example from this run:
https://github.com/AndreRicardo-Zoetis/iot-hub-device-update/actions/runs/28667644112/job/85023172815#step:10:1464

The code change with added test:
AndreRicardo-Zoetis@33fa5bf

At least one test should check the happy path with CHECK(SendCommand(validString.c_str()) == true);.