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: posix: common: separate posix semaphore tests into a standalone test #80940

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Pancakem
Copy link

@Pancakem Pancakem commented Nov 5, 2024

Fixes #80964

posix.common contains testsuites that can be separated into smaller groups of tests. This change moves semaphore into a singular testsuite at tests/posix/semaphore app directory.

@zephyrbot zephyrbot added the area: POSIX POSIX API Library label Nov 5, 2024
@Pancakem Pancakem force-pushed the origin/separate_posix_semaphores_test_into_standalone_test branch from a0647b8 to 473dbb5 Compare November 5, 2024 17:38
tests/posix/semaphore/prj.conf Outdated Show resolved Hide resolved
tests/posix/semaphore/prj.conf Outdated Show resolved Hide resolved
@Pancakem Pancakem force-pushed the origin/separate_posix_semaphores_test_into_standalone_test branch from 473dbb5 to af1f190 Compare November 6, 2024 11:02
Copy link
Member

@cfriedt cfriedt Nov 6, 2024

Choose a reason for hiding this comment

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

please rename this file to main.c and also change the suite name from semaphore to posix_semaphores to match the option group name (each of the ZTEST lines will need to be updated too)

Copy link
Author

Choose a reason for hiding this comment

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

Okay. Just to get it straight for all the tests.

  1. If it is a single source file, I should make it main.c.
  2. suite name, project name in Cmake and tag name in testcase.yaml should match the option group name.
  3. target libaries should not be included. I am curious about this, if you can shed some light on it.

Is there anything I am missing?

Copy link
Member

Choose a reason for hiding this comment

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

Okay. Just to get it straight for all the tests.

  1. If it is a single source file, I should make it main.c.

Yes, thanks.

  1. suite name, project name in Cmake and tag name in testcase.yaml should match the option group name.

Yep

  1. target libaries should not be included. I am curious about this, if you can shed some light on it.

There shouldn't be any need to add the library directory to to the application's include paths. This is mainly because the library directory (lib/posix/options) would normally only have private header, and tests should be run against public API.

Is there anything I am missing?

I think that's about it. Thanks for all of your work here 🙏

@Pancakem Pancakem force-pushed the origin/separate_posix_semaphores_test_into_standalone_test branch 3 times, most recently from 8fba314 to 1b595b3 Compare November 7, 2024 14:13
… test

posix.common contains testsuites that can be separated into smaller
groups of tests. This change moves semaphore into a singular
testsuite at tests/posix/semaphores app directory.

Signed-off-by: Marvin Ouma <[email protected]>
@Pancakem Pancakem force-pushed the origin/separate_posix_semaphores_test_into_standalone_test branch from 1b595b3 to 05fd4f7 Compare November 7, 2024 14:39
@cfriedt
Copy link
Member

cfriedt commented Nov 7, 2024

@Pancakem - please apply this diff

diff --git a/tests/posix/semaphores/testcase.yaml b/tests/posix/semaphores/testcase.yaml
index 2ad9053724c..92fdebbd992 100644
--- a/tests/posix/semaphores/testcase.yaml
+++ b/tests/posix/semaphores/testcase.yaml
@@ -7,10 +7,10 @@ common:
   platform_key:
     - arch
     - simulation
+  min_flash: 64
+  min_ram: 32
 tests:
-  portability.posix.semaphores:
-    min_flash: 64
-    min_ram: 32
+  portability.posix.semaphores: {}
   portability.posix.semaphores.minimal:
     extra_configs:
       - CONFIG_MINIMAL_LIBC=y

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: POSIX POSIX API Library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests: posix: common: split semaphore tests into a standalone test
3 participants