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

Draft: pretty-printer tests with randomly generated AST Expressions #1248

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

Draft: pretty-printer tests with randomly generated AST Expressions #1248

wants to merge 15 commits into from
+352 −9

Conversation

xpyctumo
Copy link
Contributor

@xpyctumo xpyctumo commented Dec 24, 2024 •
edited
Loading

Issue

Towards #642

Checklist

  • I have updated CHANGELOG.md
  • I have documented my contribution in docs/ and made the build locally
  • I have added tests to demonstrate the contribution is correctly implemented: this usually includes both positive and negative tests, showing the happy path(s) and featuring intentionally broken cases
  • I have run all the tests locally and no test failure was reported
  • I have run the linter, formatter and spellchecker
  • I did not do unrelated and/or undiscussed refactorings

Plan

  • AstExpressionPrimary
    • AstMethodCall
    • AstFieldAccess
    • AstStaticCall
    • AstStructInstance
    • AstNumber
    • AstBoolean
    • AstId
    • AstNull
    • AstInitOf
    • AstString
  • AstLiteral
    • AstNumber
    • AstBoolean
    • AstNull
    • AstSimplifiedString
    • AstAddress
    • AstCell
    • AstSlice
    • AstCommentValue
    • AstStructValue
  • AstOpBinary
  • AstOpUnary
  • AstConditional

@xpyctumo xpyctumo self-assigned this Dec 24, 2024
@xpyctumo xpyctumo requested a review from a team as a code owner December 24, 2024 14:27
@anton-trunov anton-trunov marked this pull request as draft December 24, 2024 14:31
@anton-trunov
Copy link
Member

anton-trunov commented Dec 24, 2024 •
edited
Loading

when I see something like this, I immediately think of adding shrinking (fast-check should support it)

(+0b100100110001001100110111100100000101101111000011101011010101101101011110010000001000101100100000011000111001001110101001111011111100001101111110011000101010010001100000100100011010000100110011001111001000111001011101000100011011100111001110 ? 0xf39c6286c40970dcb2a321a1c21e4549de1a2d60bcc7ff84622639b35024a7f || 0o3235042541620525012122214562100130425412434124566713521630100545344175751456215504064 - -0o7777777777777777777777777777777777777777777777777777777777777777777777777777777777743 : 8 - (16227299991510006730850008471880976967479204885141379564359356953639308462371 == 0o5735742350164242162734544666035326212173453606274734532135112142107426251434140545063)) ? 0b10111100010010010001011110101011101010001000111101110001100011100001101000001011001111111101110100100011001000001001001111011110100011001100011010100111110010110110010011110110010001101000100111000101011110110110110010100100010011010011 + 0b11001101101101011111001101010011101111110011111110101001101100111000110011110110011010111001101011001111001111110001100110000010001001101011100010111101000010100000010001011100101110010100100101100101101110110110111100111100110111000101011111011001100010 + 26 + (0x157e944ffa07c17e8b222faf8abfab932cd54c213784eca386a32755e232289a ? 0o2667602156054031504566507661155302000265110317032415711565650770337656725250250767327 << 0o5520154115374334170135502317743632312724505264461136361671447006177506075556352660004 : 0o6624600205764760715715350617562272506065501073701250447411343437104466044353352053407) : 0b1

even shrinking integer literals would already help a lot here

@xpyctumo
Copy link
Contributor Author

xpyctumo commented Dec 25, 2024 •
edited
Loading

when I see something like this, I immediately think of adding shrinking (fast-check should support it)

(+0b100100110001001100110111100100000101101111000011101011010101101101011110010000001000101100100000011000111001001110101001111011111100001101111110011000101010010001100000100100011010000100110011001111001000111001011101000100011011100111001110 ? 0xf39c6286c40970dcb2a321a1c21e4549de1a2d60bcc7ff84622639b35024a7f || 0o3235042541620525012122214562100130425412434124566713521630100545344175751456215504064 - -0o7777777777777777777777777777777777777777777777777777777777777777777777777777777777743 : 8 - (16227299991510006730850008471880976967479204885141379564359356953639308462371 == 0o5735742350164242162734544666035326212173453606274734532135112142107426251434140545063)) ? 0b10111100010010010001011110101011101010001000111101110001100011100001101000001011001111111101110100100011001000001001001111011110100011001100011010100111110010110110010011110110010001101000100111000101011110110110110010100100010011010011 + 0b11001101101101011111001101010011101111110011111110101001101100111000110011110110011010111001101011001111001111110001100110000010001001101011100010111101000010100000010001011100101110010100100101100101101110110110111100111100110111000101011111011001100010 + 26 + (0x157e944ffa07c17e8b222faf8abfab932cd54c213784eca386a32755e232289a ? 0o2667602156054031504566507661155302000265110317032415711565650770337656725250250767327 << 0o5520154115374334170135502317743632312724505264461136361671447006177506075556352660004 : 0o6624600205764760715715350617562272506065501073701250447411343437104466044353352053407) : 0b1

even shrinking integer literals would already help a lot here

I'm thinking about that right now. It might become more user- friendly values

value: fc.bigInt({ min: -1_000_000_000n, max: 1_000_000_000n })

@xpyctumo xpyctumo force-pushed the fast-check branch 3 times, most recently from 80ca1e6 to 88786b8 Compare December 26, 2024 17:19
@anton-trunov anton-trunov marked this pull request as ready for review December 28, 2024 08:57
@anton-trunov anton-trunov added this to the v1.6.0 milestone Dec 28, 2024
@anton-trunov
Copy link
Member

@xpyctumo I took the liberty to merge the new parser into your branch and pushed a few fixes for the expressions generator too. Looks like the new parser fixed the issue with duplicated unary operators!

@anton-trunov anton-trunov changed the title Draft: pretty-printer tests with random generated AST Expressions Draft: pretty-printer tests with randomly generated AST Expressions Dec 28, 2024
@anton-trunov
Copy link
Member

Btw, these fuzzing-like tests should go into their own folder and only gets executed on a special request, separately from yarn tests.

@xpyctumo xpyctumo force-pushed the fast-check branch 2 times, most recently from f5d0f1f to ccdf98c Compare January 14, 2025 15:46
anton-trunov
anton-trunov reviewed Jan 16, 2025
View reviewed changes
src/test/prettyPrint/expressions.spec.ts Outdated
const maxDepth = 3;
const expression = () => randomAstExpression(maxDepth);

const cases: [string, fc.Arbitrary<AstExpression>][] = [
Copy link
Member

Choose a reason for hiding this comment

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

looks like you don't need cases here: you just need to generate a lot of random expressions, using randomAstExpression: it will handle all the cases internally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want to generate one thing?

I left it out specifically so that you can selectively do tests on each subset

Copy link
Member

Choose a reason for hiding this comment

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

yep, expressions are a fine level of granularity, given that you also need to generate functions with statements, contracts etc

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 removed the list of cases, but increased the number of runs

@xpyctumo xpyctumo linked an issue Jan 20, 2025 that may be closed by this pull request
AST-based randomized testing of Tact parser/pretty-printer #642
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anton-trunov anton-trunov anton-trunov left review comments

@verytactical verytactical Awaiting requested review from verytactical

At least 1 approving review is required to merge this pull request.

Assignees

@anton-trunov anton-trunov

@verytactical verytactical

Labels
None yet
Projects
None yet
Milestone
v1.6.0
Development

Successfully merging this pull request may close these issues.

AST-based randomized testing of Tact parser/pretty-printer
3 participants
@xpyctumo @anton-trunov @verytactical