Skip to content
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

Introduce scripts twister extended #20544

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nordic-piks
Copy link
Contributor

@nordic-piks nordic-piks commented Feb 21, 2025

This is an extension for aligning zephyr tests for new platforms, which are under preparation, especially those at pre-silicon stage.
For that we want to enable multiple tests which can allow as to validate HW more.
Extending them at native zephyr is slow and adds additional, unwanted process complexity.

This approach allows to do small adjustments for samples/tests - like editing sample/testcase.yaml, adding overlays and confs. Will not edit code in this way, thus fixes and adjustments in the drivers should be done directly at zephyr.

@nordic-piks nordic-piks requested review from a team as code owners February 21, 2025 07:48
@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 Feb 21, 2025
Copy link

github-actions bot commented Feb 21, 2025

After documentation is built, you will find the preview for this PR here.

Preview links for modified nRF Connect SDK documents:

https://ncsdoc.z6.web.core.windows.net/PR-20544/nrf/releases_and_maturity/releases/release-notes-changelog.html

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Feb 21, 2025

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 12

Inputs:

Sources:

sdk-nrf: PR head: 654d0940720c688339f1aeab95eb0f5bb94b1669

more details

sdk-nrf:

PR head: 654d0940720c688339f1aeab95eb0f5bb94b1669
merge base: 5c21704fe090b0f51eb4d341149df208188d03b1
target head (main): a95e127c906afef92e36e79120d2ec127c250b87
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (13)
.github
│  │ test-spec.yml
CODEOWNERS
samples
│  ├── sensor
│  │  ├── qdec
│  │  │  ├── CMakeLists.txt
│  │  │  ├── README.txt
│  │  │  ├── boards
│  │  │  │  │ nrf54l20pdk_nrf54l20_cpuapp.overlay
│  │  │  ├── prj.conf
│  │  │  │ sample.yaml
│  ├── subsys
│  │  ├── settings
│  │  │  ├── CMakeLists.txt
│  │  │  ├── README.txt
│  │  │  ├── boards
│  │  │  │  │ nrf54l20pdk_nrf54l20_cpuapp.conf
│  │  │  ├── prj.conf
│  │  │  │ testcase.yaml
scripts
│  ├── ci
│  │  │ tags.yaml

Outputs:

Toolchain

Version: aedb4c0245
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:aedb4c0245_bece0367df

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 34
  • 🟠 Integration tests
    • 🟠 test-low-level
Disabled integration tests
    • desktop52_verification
    • doc-internal
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-boot
    • test-fw-nrfconnect-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_cloud
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_mosh
    • test-fw-nrfconnect-nrf-iot_positioning
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-proprietary_esb
    • test-fw-nrfconnect-ps
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread
    • test-fw-nrfconnect-zigbee
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-sdk-sidewalk
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

@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 Feb 21, 2025
@nordic-piks nordic-piks requested review from a team February 21, 2025 07:53
@nordic-piks
Copy link
Contributor Author

@nrfconnect/ncs-ll-ursus this approach can be more easy to use than alt config.
It supports west build and prevent overriding original yaml file, thus reducing maintenance cost here.
You need to keep prj aligned and cmake, but this may be lighter.

With this approach it will be also easy to add it globally into CI, not only LL.

@nordic-piks nordic-piks force-pushed the introduce_scripts_twister_extended branch 3 times, most recently from 485737c to c2df84d Compare February 21, 2025 08:20
@PerMac
Copy link
Contributor

PerMac commented Feb 21, 2025

@nrfconnect/ncs-co-build-system do you know if a prj.conf from original source can be linked without the need to copy it?

@nordic-segl
Copy link
Contributor

  1. Symbolic link but drawback is that's platform dependent. The "copied' sample will not build on Windows.
  2. Maybe, extend sample.yaml with extra_args: CONF_FILE=[path]; This will require to build the sample with west build --test-item xyz .

@nordic-piks
Copy link
Contributor Author

  1. Symbolic link but drawback is that's platform dependent. The "copied' sample will not build on Windows.
  2. Maybe, extend sample.yaml with extra_args: CONF_FILE=[path]; This will require to build the sample with west build --test-item xyz .

I was thinking about solution described at 2 ie. did not find sth better.

@nordic-piks nordic-piks requested review from katgiadla and a team as code owners February 21, 2025 09:30
integration_platforms:
- nrf54l20pdk/nrf54l20/cpuapp
harness: console
harness_config:
Copy link
Contributor

Choose a reason for hiding this comment

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

can use base prj.conf with

    extra_args:
      - EXTRA_CONF_FILE=boards/nrf54l20pdk_nrf54l20_cpuapp.conf
      - CONF_FILE="${ZEPHYR_base}/samples/subsys/settings/prj.com"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added CONF_FILE as you suggested, thus prj.conf now are taken from original place by default, thus maintenance should be easier, thanks :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like adding CONF_FILE makes other conf and overlays (from boards) not applied. thus, for simplicity, I will stay with copying proj.conf.

Copy link
Contributor

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

I find it very wrong to start placing CMakeLists.txt, conf file, overlays, etc under: scripts/twister/extended.

This is really messing up where to find things.
We should have some dedicated test folder location.
if tests cannot be used directly due to naming conflicts, then perhaps tests/extend.

I'm open for alternatives, but definitely not under scripts.

@nordic-piks
Copy link
Contributor Author

I find it very wrong to start placing CMakeLists.txt, conf file, overlays, etc under: scripts/twister/extended.

This is really messing up where to find things. We should have some dedicated test folder location. if tests cannot be used directly due to naming conflicts, then perhaps tests/extend.

I'm open for alternatives, but definitely not under scripts.

There is already accepted by Carles location for very similar things - alt-configs - at https://github.com/nrfconnect/sdk-nrf/tree/main/scripts/twister/alt, thus I created my "extenstion" here.

We want to keep structure of tests and samples exact as in zephyr, thus we cannot use nrf/tests or nrf/samples for that, as this will make more chaos.

Please note that this is only done to make testing faster, we do not want to expose that we support those tests and samples in this way - at the end, they should be officially added for support at zephyr. Then this, a little bit hidden folder, is IHMO very good place.

@nordic-piks nordic-piks force-pushed the introduce_scripts_twister_extended branch from d214145 to 4c06914 Compare February 21, 2025 11:28
@nordic-piks nordic-piks requested a review from tejlmand February 21, 2025 11:30
@nordic-piks nordic-piks force-pushed the introduce_scripts_twister_extended branch 2 times, most recently from a277d91 to 553f862 Compare February 21, 2025 14:22
@nordic-piks nordic-piks requested a review from a team as a code owner February 21, 2025 14:22
@github-actions github-actions bot added the doc-required PR must not be merged without tech writer approval. label Feb 21, 2025
@nordic-piks nordic-piks force-pushed the introduce_scripts_twister_extended branch from 553f862 to e3bf935 Compare February 21, 2025 15:59
Extend configuration for zephyr/samples/sensor/qdec
for nrf54L20.

Signed-off-by: Piotr Kosycarz <[email protected]>
…f54L20

Extend configuration for zephyr/samples/subsys/settings
for nrf54L20.

Signed-off-by: Piotr Kosycarz <[email protected]>
Initially ncs-low-level-test.

Signed-off-by: Piotr Kosycarz <[email protected]>
ci_samples_sensor_qdec
ci_samples_subsys_settings

Signed-off-by: Piotr Kosycarz <[email protected]>
Add dependency for LL testing.

Signed-off-by: Piotr Kosycarz <[email protected]>
@nordic-piks nordic-piks force-pushed the introduce_scripts_twister_extended branch from e3bf935 to 654d094 Compare February 24, 2025 06:48
@github-actions github-actions bot removed the doc-required PR must not be merged without tech writer approval. label Feb 24, 2025
@nordic-piks nordic-piks removed request for a team, umapraseeda and divipillai February 24, 2025 06:53
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.

7 participants