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

github: workflows: Add test for offloaded sockets #76

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

Conversation

jerome-pouiller
Copy link
Contributor

Offloaded sockets can be easily broken. This patch ensures it won't happen.

@jerome-pouiller jerome-pouiller marked this pull request as ready for review December 20, 2024 16:29
@jerome-pouiller jerome-pouiller requested a review from a team as a code owner December 20, 2024 16:29
@@ -76,3 +76,4 @@ jobs:
shell: bash
run: |
west twister -s sample.net.wifi -p siwx917_rb4338a -v --inline-logs -K
west twister -s sample.net.wifi -p siwx917_rb4338a -x EXTRA_CONF_FILE=$(pwd)/samples/net/wifi/shell/soc/siwx917_offloaded_sockets.conf -v --inline-logs -K
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose the clean way to do this would be to define a new test case in sample.yaml (call it e.g. sample.net.wifi.offload) - I suppose that's not an option for now since we've so far avoided forking the main tree? Also, generally, let's try to start splitting up these long lines with the help of \ as has been done elsewhere.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, except it seems the test is in the same tree in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sample.net.wifi come from upstream. Only siwx917_offloaded_sockets.conf is located in the downstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose the clean way to do this would be to define a new test case in sample.yaml

Yes, I expect to do that when support for 917 will be merged.

Offloaded sockets can be easily broken. This patch ensures it won't
happen.

Signed-off-by: Jérôme Pouiller <[email protected]>
Just cut long line.

Signed-off-by: Jérôme Pouiller <[email protected]>
Currently, the variable "TC_OVERLAY_FILE" was used instead of
"DTC_OVERLAY_FILE" and options "-v" and "--inline-logs" were passed to
"west build" instead of twister.

In addition, the other test use "-x" rather than "--" to pass option to
"west build"

Signed-off-by: Jérôme Pouiller <[email protected]>
This is yet another cosmetic change. The idea is to make the tests
easier to read by placing each test and each platform on one line.

Signed-off-by: Jérôme Pouiller <[email protected]>
@jerome-pouiller jerome-pouiller force-pushed the add-test-for-offloaded-sockets branch from 3761ee2 to a063c5b Compare December 21, 2024 11:08
@jerome-pouiller
Copy link
Contributor Author

v2:

  • break long lie
  • also break long lines in existing tests
  • fix DMA tests
  • suggest a new presentation for the invocation of twister

@jhedberg, do you think it is a good idea, or you have some suggestion to improve it? At the end, I hope to have only one invocation of twister for all the test and all the boards. But I believe it is still not yet possible with our downstream.

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.

2 participants