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

Expand STM Semaphore tests #419

Merged
merged 3 commits into from
Dec 8, 2023
Merged

Expand STM Semaphore tests #419

merged 3 commits into from
Dec 8, 2023

Conversation

jmid
Copy link
Collaborator

@jmid jmid commented Dec 7, 2023

While trying out a run of multicoretests under TSan, I spotted some limitations of our STM Semaphore tests.
For one, we test Semaphore.Counting.get_value in parallel despite it being documented as racy.

This PR

  • includes a parallel variant of the existing STM Semaphore.Counting test that excludes get_value
  • adds a corresponding Semaphore.Binary STM test for completeness
  • renames files accordingly

Because of the precondition in the Semaphore.Binary STM test, it needs to generate extra inputs.
By default, qcheck-stm tries to generate up to 3x as many inputs as needed which is not quite enough to hit 500:

$ dune exec src/semaphore/stm_tests_binary.exe -- -v
random seed: 338133583
generated error fail pass / total     time test name
[✓]  500    0    0  500 /  500     0.1s STM Semaphore.Binary test sequential
[✓] 1500    0    0  332 /  500     2.1s STM Semaphore.Binary test parallel
================================================================================
success (ran 2 tests)

The test is still relatively fast with a count of 500 compared to the STM Semaphore.Counting test
which has a count of 200:

$ dune exec src/semaphore/stm_tests.exe -- -v
random seed: 72247306
generated error fail pass / total     time test name
[✓]  200    0    0  200 /  200     0.0s STM Semaphore.Counting test sequential
[✓]  455    0    0  200 /  200     2.1s STM Semaphore.Counting test parallel (w/ get_value)
[✓]  600    0    0  179 /  200     1.7s STM Semaphore.Counting test parallel (w/o get_value)
================================================================================
success (ran 3 tests)

As such I'm leaning towards declaring good enough...

@jmid
Copy link
Collaborator Author

jmid commented Dec 8, 2023

CI summary:

Out of 63 workflows 3 failed with 1 genuine error and 2 false positives

@jmid jmid merged commit df41332 into main Dec 8, 2023
50 of 53 checks passed
@jmid jmid deleted the semaphore-expand branch December 8, 2023 11:38
@jmid
Copy link
Collaborator Author

jmid commented Dec 11, 2023

CI summary for merge to main:

Out of 38 workflows 5 failed with 2 genuine issues, 2 CI issue, and 1 test suite false alarm.

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.

1 participant