Skip to content

Conversation

@eivindj-nordic
Copy link
Contributor

@eivindj-nordic eivindj-nordic commented Jan 7, 2026

Place unit tests in tests/unit/ folder, other tests in tests/ folder.

@eivindj-nordic eivindj-nordic self-assigned this Jan 7, 2026
@eivindj-nordic eivindj-nordic requested review from a team and rghaddab as code owners January 7, 2026 08:54
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jan 7, 2026
@github-actions
Copy link

github-actions bot commented Jan 7, 2026

You can find the documentation preview for this PR here.

@eivindj-nordic eivindj-nordic requested a review from a team as a code owner January 7, 2026 09:00
@github-actions github-actions bot removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jan 7, 2026
@eivindj-nordic eivindj-nordic requested a review from a team as a code owner January 7, 2026 09:36
@eivindj-nordic eivindj-nordic force-pushed the test_split branch 3 times, most recently from 31039b7 to 80c241c Compare January 7, 2026 09:51
@fundakol
Copy link
Contributor

fundakol commented Jan 7, 2026

The testcase.yaml file may include multiple harnesses, not limited to just pytest. For instance, tests/pytest/subsys/kmu/hello_for_kmu/testcase.yaml contains both pytest and console harnesses. Therefore, moving these tests to the pytest folder is not sensible, as we run all these tests using twister, not with pytest. Also there is no such split in zephyr-rtos and nrf-sdk.

@eivindj-nordic
Copy link
Contributor Author

The testcase.yaml file may include multiple harnesses, not limited to just pytest. For instance, tests/pytest/subsys/kmu/hello_for_kmu/testcase.yaml contains both pytest and console harnesses. Therefore, moving these tests to the pytest folder is not sensible, as we run all these tests using twister, not with pytest. Also there is no such split in zephyr-rtos and nrf-sdk.

You are correct there is no such split in zephyr, sdk-nrf etc. The reason we want to split it is that it gives an easier overview of the tests we have, e.g. what modules we have unit tests for, and what we have system tests (ztest, pytest ...) for. Also, there might be modules where we have both unit tests and system tests, and it is not very practical putting them in the same location. Of course the testcase.yaml file will have twister pick up the correct tests regardless. Twister can be used to run all the tests.

I'm open for other suggestions on how to structure it, though I don't think keeping it as it is is ideal.

@PerMac
Copy link

PerMac commented Jan 7, 2026

I agree with @fundakol on this. To me it is more useful and functional to have tests grouped by the relevant area than by a specific harness (framework) they use. If I am looking for bluetooth related stuff I would prefer to have them all in mutual directory than scattered across multiple places and trying to figure out what is the difference between ztest/console/pytest.
I think when we are talking about tests coverage we first think about the component/area and how well with what king of tests we have it covered, rather then "where we have unit test". In other words, first comes "what?" and only then "how". It is also easier from maintenance perspective where we work in multiple groups responsible for multiple areas. We don't have any group dedicated to unit tests and another to system ones.
Also as pointed by @fundakol harness in twister is not the best indicator of test type. One can have both unit level tests in ztests and system.

@lemrey
Copy link
Contributor

lemrey commented Jan 7, 2026

Would it be fine to split just tests/unity and tests/ztest?

@eivindj-nordic
Copy link
Contributor Author

eivindj-nordic commented Jan 8, 2026

Would it be fine to split just tests/unity and tests/ztest?

I don't think unity and ztest would be the correct terms then, though perhaps test/unit and test/system or so?
@MechanicV1 How would that work for your integration? How would it work leaving it as it is?

@MechanicV1
Copy link
Contributor

Splitting it to unit/ and system/ seems cleaner !

@eivindj-nordic
Copy link
Contributor Author

Split the tests into unit and system instead of based on the test harness. That should allow for tests using multiple harnesses, while keeping a clear distinction of what is unit tested and what is system tested.

CODEOWNERS Outdated
Comment on lines 114 to 115
/tests/system/subsys/fs/bm_zms/ @nrfconnect/ncs-bm @rghaddab
/tests/system/subsys/kmu/ @nrfconnect/ncs-eris
Copy link
Contributor

Choose a reason for hiding this comment

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

-1 on system subfolder, have it /tests/subsys/X

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other suggestions are welcome. Point is that we want to have one folder with unit tests and one folder with system tests. There will be more system tests to follow, not only in subsys.
Corrected codeowners file alignment.

Copy link
Contributor

Choose a reason for hiding this comment

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

the unit tests go in /tests/unit the other tests go in /tests just as they do in ncs and zephyr

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, I would suggest we follow the tests/unit/ and tests/ to match NCS and Zephyr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I was not aware of that pattern in NCS. On a side note, it seems that some cleanup is required in sdk-nrf (search CONFIG_UNITY=y). The tests are not within that folder.

Will update the PR.

Place unit tests in unit folder, and system tests in system folder.

Signed-off-by: Eivind Jølsgard <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants