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

tests: refactor tests #649

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

Conversation

ktaborowski
Copy link
Contributor

@ktaborowski ktaborowski commented Nov 29, 2024

KRKNWK-14886
Move and refactor tests:

  • critical_region
  • crypto
  • crypto_keys
  • delay
  • interrupts
  • mfg not working
  • spi
  • storage
  • temperature
  • time

CI parameters

Github_actions:
  #(branch, hash, pull/XXX/head)
  NRF_revision: main

  # Do not change after creating PR
  Create_NRF_PR: false
Jenkins:
  test-sdk-sidewalk: master

Description

JIRA ticket:

Self review

  • There is no commented code.
  • There are no TODO/FIXME comments without associated issue ticket.
  • Commits are properly organized.
  • Change has been tested.
  • Tests were updated (if applicable).

@github-actions github-actions bot added source PR changing src files tests labels Nov 29, 2024
Copy link

Sample diff used total

Memory usage did not change for any of the samples.

@ktaborowski ktaborowski force-pushed the tests_new branch 2 times, most recently from b45a9c4 to 9c04745 Compare December 2, 2024 15:40
@github-actions github-actions bot added the doc-required PR must not be merged without tech writer approval. label Dec 2, 2024
@ktaborowski ktaborowski force-pushed the tests_new branch 2 times, most recently from b9d68f7 to ddcf163 Compare December 9, 2024 09:12
@ktaborowski ktaborowski force-pushed the tests_new branch 3 times, most recently from 1906f7c to 28e2d02 Compare December 16, 2024 08:46
@ktaborowski ktaborowski marked this pull request as ready for review December 16, 2024 08:56
Copy link
Collaborator

@RobertGalatNordic RobertGalatNordic left a comment

Choose a reason for hiding this comment

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

crypto
mfg
spi
storage

Require rewriting or major refactoring. Please move them to their old location, as they will be addressed in separate PRs.

If some test does not pass, please skip it and place it back in its old location (move to tests_new, only refactored/approved and working tests)

tests_new/integration/time/src/main.c Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

refactor tests, tests are coupled, need to be run in order, commented test

Copy link
Collaborator

Choose a reason for hiding this comment

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

/**

  • Test spi transfer, reuse external flash because it is already on DK? otherwise this test will require some fixture to run

*/

Copy link
Collaborator

Choose a reason for hiding this comment

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

/**
TODO rewrite the test

read suite:
v7
v8

write suite:

clear memory,
write values
read values
compare

persistant test
write values
dump flash to different address
initialize on new address (copy)
read values
compare

*/

Copy link
Collaborator

Choose a reason for hiding this comment

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

#if defined(CONFIG_SOC_SERIES_NRF54LX)
// TODO: this ifdef should be based on feature not SOC used. replace it by feature: SID_PAL_HASH_SHA512 check

/**
I want to see what configurations were tested. Maybe some kind of matrix that show what function with what algorithms, and what values were tested
what error cases were covered

each function should have its own suite maybe some kind of macro generation of test functions would be usefull to make this information more compact

maybe separating functions, into separate files would be benefitial

maybe some defines, should be moved to Kconfig of the test, or separate configuration header.
Simmilarly there are many schemes like test_data, correct_response. extracting those long data arrays would make tests more redeable.
*/

tests_new/integration/crypto_keys/src/main.c Show resolved Hide resolved
tests_new/integration/delay/src/main.c Show resolved Hide resolved
tests_new/integration/interrupts/src/main.c Show resolved Hide resolved
tests_new/integration/mfg/testcase.yaml Outdated Show resolved Hide resolved
[KRKNWK-14886]

* Move critical_region test to new location
* Move crypto keys test to new location
* Move crypto test to new location
* Move delay test to new location
* Move interrupts test to new location
* Move spi test to new location
* Move storage test to new location
* Move temperature test to new location
* Move time test to new location

* Add native_sim board support (not all)
* Port to ztest
* Remove sanity tests

Signed-off-by: Krzysztof Taborowski <[email protected]>
Tests skipped - to be rewritten

Signed-off-by: Krzysztof Taborowski <[email protected]>
Use mock temperature pal when no temp sensor in dts

Signed-off-by: Krzysztof Taborowski <[email protected]>
to be squashed

Signed-off-by: Krzysztof Taborowski <[email protected]>
to be fixed later

Signed-off-by: Krzysztof Taborowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-required PR must not be merged without tech writer approval. source PR changing src files tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants