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.

Sign up for GitHub

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

Jump to bottom

Enable nothunk tests by setting check-tvar-invariant flag for strict-stm #4002

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Enable nothunk tests by setting check-tvar-invariant flag for strict-stm #4002

wants to merge 9 commits into from
+45 −1

Conversation

yogeshsajanikar
Copy link
Contributor

@yogeshsajanikar yogeshsajanikar commented Sep 13, 2022 •
edited by dnadales
Loading

Description

  • Add a flag checkTVarInvariant to ./nix/ouroboros-network.nix
  • haskellPackagesWithTVarCheck points to strict-stm package with "checktvarinvariant" flag enabled.
  • nix-build -A tvar-invariant-checks.tests runs the tests with "checktvarinvariant" flag turned on
  • Github action "Strict NoThunk Test" should run the tests with above flag turned on

Closes IntersectMBO/ouroboros-consensus#604

Checklist

  • Branch
    • Commit sequence broadly makes sense
    • Commits have useful messages
    • New tests are added if needed and existing tests are updated
    • If this branch changes Consensus and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
    • If this branch changes Network and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
    • If serialization changes, user-facing consequences (e.g. replay from genesis) are confirmed to be intentional.
  • Pull Request
    • Self-reviewed the diff
    • Useful pull request description at least containing the following information:
      • What does this PR change?
      • Why these changes were needed?
      • How does this affect downstream repositories and/or end-users?
      • Which ticket does this PR close (if any)? If it does, is it linked?
    • Reviewer requested

@yogeshsajanikar
Add checkTVarInvariant flag
473f9a2
@yogeshsajanikar
Add tvar invariant check
dac97e8 
- TVar invariant check for enabling strict, nothunk tests
- Add github action to build and test
@yogeshsajanikar
Correct spelling
8221d93
@yogeshsajanikar
Enable github action for this branch
e50f547 
- This is a temporary change and should be reverted before pushing into
master
amesgen
amesgen requested changes Sep 13, 2022
View reviewed changes
.github/workflows/check-invariant-test.yml Outdated Show resolved Hide resolved
.github/workflows/check-invariant-test.yml Outdated Show resolved Hide resolved
.github/workflows/check-invariant-test.yml Outdated Show resolved Hide resolved
.github/workflows/check-invariant-test.yml Outdated Show resolved Hide resolved
.github/workflows/check-invariant-test.yml Outdated Show resolved Hide resolved
.github/workflows/check-invariant-test.yml
@@ -0,0 +1,40 @@
name: Strict NoThunk test
on:
Copy link
Member

Choose a reason for hiding this comment

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

Before this is merged, a schedule trigger should be added here, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes!

nix/ouroboros-network.nix Outdated Show resolved Hide resolved
yogeshsajanikar and others added 2 commits September 13, 2022 15:01
@yogeshsajanikar @amesgen
Update nix/ouroboros-network.nix
674442c 
Co-authored-by: amesgen <[email protected]>
@yogeshsajanikar
Correct github action versions
5e6671d 
- Use latest github action version
- Keep the test running even after failures
amesgen
amesgen reviewed Sep 14, 2022
View reviewed changes
.github/workflows/check-invariant-test.yml Outdated
with:
extra_nix_config: |
trusted-public-keys = cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY= hydra.iohk.io:f/Ea+s+dFdN+3Y/G+FDgSq+a5NEWhJGzdjvKNGv0/EQ=
substituters = https://cache.nixos.org https://cache.iog.io https://iohk.cachix.org
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
substituters = https://cache.nixos.org https://cache.iog.io https://iohk.cachix.org
substituters = https://cache.nixos.org https://cache.iog.io

@yogeshsajanikar yogeshsajanikar marked this pull request as ready for review September 15, 2022 07:34
@yogeshsajanikar yogeshsajanikar self-assigned this Sep 15, 2022
@yogeshsajanikar yogeshsajanikar marked this pull request as draft September 20, 2022 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amesgen amesgen amesgen requested changes

@coot coot Awaiting requested review from coot

@newhoggy newhoggy Awaiting requested review from newhoggy

Requested changes must be addressed to merge this pull request.

Assignees

@yogeshsajanikar yogeshsajanikar

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Check the NoThunks invariant during a nightly CI test
2 participants
@yogeshsajanikar @amesgen