-
Notifications
You must be signed in to change notification settings - Fork 3k
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
CMake: unittests: Show all tests in CTest report #14944
Conversation
@rwalton-arm, thank you for your changes. |
This is still a work in progress, I have the following test failures still to fix (previously the failures were being masked because we only showed them as failed if the binary returned an error code):
|
2090287
to
9b7d1f0
Compare
bbfb48b
to
ccb9271
Compare
Previously a test executable was recognised as a single test by CTest. However, test executables usually contain multiple test cases, the results of the test cases should be individually reported. With our previous setup we could miss test case failures that didn't cause the executable to return an error code. This commit uses gtest_discover_test to discover all test cases in a test executable. This enables CTest to match test passes and failures from the googletest binary output.
Some tests in athandlertest.cpp relied on the state of stubs set in previous tests. This caused test failures if the tests cases were ran in isolation. Remove the interdependencies between tests by setting stub values in the tests that rely on them.
…init_get Instead of performing 10,000 "set, deinit, get" operations, let's just perform 100. This reduces test time from 4.8s to 0.02s.
We had commented out a line where we reset LoRaPHY_stub::uint16_value to 0. This was causing an invalid array access in LoRaMac::handle_data_frame, when trying to extract the _mps_indication.channel from list of channel_params_t returned by lora_phy->get_phy_channels.
This test was relying on the state of `LoRaMac_stub::bool_true_counter` to be set to 1 from a previous test. When the test was ran in isolation the `LoRaWANStack::rx_timeout_interrupt_handler` callback would fail because we weren't returning true from the stub implementation of `LoRaMac::nwk_joined`. Due to this `LoRaWANStack::post_process_tx_no_reception` was never called. This caused the LoRaWANStack object to think the "tx_metadata" was "stale" (i.e the data hadn't changed since any previous read). So, when we attempted to call `LoRaWANStack::acquire_tx_metadata` it returned `LORAWAN_STATUS_METADATA_NOT_AVAILABLE` as it thought we had no new metadata to report, causing the test assertion to fail.
We failed to set `LoRaMac_stub::mlme_ind_ptr` to a valid pointer. When we tried to dereference it in `LoRaWANStack::process_reception` we hit a SEGFAULT.
94c527c
to
98e25f1
Compare
Ready for review now. |
Jenkins CI Test : ✔️ SUCCESSBuild Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & ArtifactsCLICK for Detailed Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
CI Started
This PR does not contain release version label after merging. |
Version fixed |
Summary of changes
Previously a test executable was recognised as a single test by CTest.
However, test executables usually contain multiple test cases, the
results of the test cases should be individually reported. With our
previous setup we could miss test case failures that don't cause the
executable to return an error code.
This commit uses gtest_discover_test to discover all test cases in a
test executable. This enables CTest to match test passes and failures
from the googletest binary output.
This PR also fixes some tests which were dependent on the state of
mocks set in previous tests, meaning they had to be executed in a specific
order or the tests would fail.
Impact of changes
Migration actions required
Documentation
Pull request type
Test results
Reviewers