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

Don't let doctest quirks stop testing in release mode #60

Merged
merged 3 commits into from
Feb 28, 2024

Conversation

dvdplm
Copy link
Contributor

@dvdplm dvdplm commented Feb 23, 2024

On main we can't run tests with a simple cargo t --release because of a quirk in the way doctests are compiled in Rust. Doctests do not "see" compilation attributes like e.g. test, doctest or, as is the case here, debug_assertions because they are treated as separate compilation units.

To work around this, this PR moves the relevant use clauses to the one spot in the code where the debug_utils modules are actually used and removes the conditional compilation from the modules themselves. The same has to be done in starky and that must be merged and published before this PR can be merged.

Depends on 0xPolygonZero/plonky2#1540 being merged and published.

@dvdplm dvdplm marked this pull request as ready for review February 23, 2024 16:27
Copy link
Collaborator

@Nashtare Nashtare left a comment

Choose a reason for hiding this comment

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

This is fine to me. However, the issue was coming from starky hiding behind a conditional debug_assertions flag the module to be imported here, which is what was messing up doctest runs.
As starky v.0.2.1 would expose this module regardless, there are technically no changes needed here to make it work. So I'm not sure we should bother (although having the imports within the conditionally evaluated brackets seems cleaner to me).

@dvdplm dvdplm self-assigned this Feb 28, 2024
@dvdplm dvdplm enabled auto-merge (squash) February 28, 2024 11:55
@dvdplm dvdplm merged commit 6f1438a into main Feb 28, 2024
5 checks passed
@dvdplm dvdplm deleted the dp-allow-tests-in-release-mode branch February 28, 2024 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants