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

PLT-1398 Set up typechecking property tests for all PIR compiler passes #5565

Merged
merged 29 commits into from
Oct 12, 2023
Merged

PLT-1398 Set up typechecking property tests for all PIR compiler passes #5565

merged 29 commits into from
Oct 12, 2023

Conversation

thealmarty
Copy link
Contributor

@thealmarty thealmarty commented Sep 28, 2023 •
edited
Loading

Add typechecking property tests to each simplifier pass and the overall simplifier pass.

Some of the code is a bit repetitive but I think it's ok. This way it's tidy and easy to follow which tests are which.

The inline and overall simplifier passes are failing atm so they're marked as expected to fail. Debugging to follow in another PR. See ticket.

Pre-submit checklist:

  • Branch
    • Tests are provided (if possible)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Changelog fragments have been written (if appropriate)
    • Relevant tickets are mentioned in commit messages
    • Formatting, PNG optimization, etc. are updated
  • PR
    • (For external contributions) Corresponding issue exists and is linked in the description
    • Targeting master unless this is a cherry-pick backport
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

@thealmarty thealmarty changed the title Thealmarty/plt 1398 pir typechecking tests PLT-1398 Set up typechecking property tests for all PIR compiler passes Sep 28, 2023
@thealmarty thealmarty requested a review from a team September 28, 2023 17:25
@thealmarty thealmarty marked this pull request as ready for review September 28, 2023 17:28
@thealmarty thealmarty marked this pull request as draft September 28, 2023 17:36
@thealmarty thealmarty force-pushed the thealmarty/plt-1398-pir-typechecking-tests branch from a7d7ad6 to 246c4c9 Compare September 28, 2023 17:40
@thealmarty thealmarty removed the request for review from a team September 28, 2023 17:40
@thealmarty
Copy link
Contributor Author

I'll be adding other PIR compiler passes. Feel free to look at the tests for the simplifier passes.

michaelpj
michaelpj reviewed Sep 29, 2023
View reviewed changes
plutus-core/plutus-ir/test/TypecheckSpec.hs Outdated Show resolved Hide resolved
plutus-core/plutus-ir/test/TypecheckSpec.hs Outdated Show resolved Hide resolved
plutus-core/plutus-ir/test/TypecheckSpec.hs Outdated Show resolved Hide resolved
plutus-core/plutus-ir/test/TypecheckSpec.hs Outdated Show resolved Hide resolved
plutus-core/plutus-ir/test/TypecheckSpec.hs Outdated Show resolved Hide resolved
plutus-core/plutus-ir/test/TypecheckSpec.hs Outdated Show resolved Hide resolved
plutus-core/plutus-ir/test/TypecheckSpec.hs Outdated Show resolved Hide resolved
plutus-core/plutus-ir/test/TypecheckSpec.hs Outdated Show resolved Hide resolved
plutus-core/plutus-ir/test/TypecheckSpec.hs Outdated Show resolved Hide resolved
@thealmarty thealmarty force-pushed the thealmarty/plt-1398-pir-typechecking-tests branch 9 times, most recently from c1a5e7b to fcd895e Compare October 5, 2023 15:18
@thealmarty thealmarty marked this pull request as ready for review October 5, 2023 15:21
@thealmarty thealmarty requested a review from a team October 5, 2023 15:21
@thealmarty
Copy link
Contributor Author

As discussed I've changed the tests to testing that the terms typecheck after a pass. Fewer tests are failing but still there are some. They are marked as expected failures and will be investigated in another PR (see ticket).

michaelpj
michaelpj reviewed Oct 6, 2023
View reviewed changes
plutus-core/plutus-ir/test/PlutusIR/Transform/CaseReduce/Test.hs Outdated Show resolved Hide resolved
plutus-core/plutus-ir/test/PlutusIR/Transform/DeadCode/Tests.hs Outdated Show resolved Hide resolved
plutus-core/plutus-ir/test/PlutusIR/Transform/DeadCode/Tests.hs Outdated Show resolved Hide resolved
plutus-core/plutus-ir/test/PlutusIR/Transform/EvaluateBuiltins/Tests.hs Outdated Show resolved Hide resolved
plutus-core/plutus-ir/test/PlutusIR/Transform/LetFloatIn/Tests.hs Outdated Show resolved Hide resolved
plutus-core/testlib/PlutusIR/Properties/Typecheck.hs Outdated Show resolved Hide resolved
plutus-core/testlib/PlutusIR/Properties/Typecheck.hs Outdated Show resolved Hide resolved
plutus-core/testlib/PlutusIR/Properties/Typecheck.hs Outdated Show resolved Hide resolved
plutus-core/testlib/PlutusIR/Properties/Typecheck.hs Show resolved Hide resolved
plutus-core/testlib/PlutusIR/Properties/Typecheck.hs Show resolved Hide resolved
@thealmarty thealmarty force-pushed the thealmarty/plt-1398-pir-typechecking-tests branch 5 times, most recently from 64d6667 to f36d6fc Compare October 11, 2023 17:20
thealmarty
thealmarty commented Oct 11, 2023
View reviewed changes
plutus-core/plutus-ir/test/PlutusIR/Compiler/Let/Test.hs
-- FIXME
-- | Check that a term typechecks after a
-- `PlutusIR.Compiler.Let.compileLets` (recursive terms) pass.
prop_TypecheckCompileLetsRecTerms :: Property
Copy link
Contributor Author

@thealmarty thealmarty Oct 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I could let it generate random LetKind too but I feel in this case it's more obviously useful to separate them. Let me know if you think otherwise.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, in this case it actually means "do something completely different", so I think it's good to separate them.

@thealmarty thealmarty force-pushed the thealmarty/plt-1398-pir-typechecking-tests branch from f36d6fc to cbe39f5 Compare October 11, 2023 17:39
michaelpj
michaelpj approved these changes Oct 12, 2023
View reviewed changes
plutus-core/plutus-ir/test/PlutusIR/Compiler/Let/Test.hs
-- FIXME
-- | Check that a term typechecks after a
-- `PlutusIR.Compiler.Let.compileLets` (recursive terms) pass.
prop_TypecheckCompileLetsRecTerms :: Property
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, in this case it actually means "do something completely different", so I think it's good to separate them.

plutus-core/plutus-ir/test/PlutusIR/Compiler/Let/Test.hs
-- `PlutusIR.Compiler.Let.compileLets` (data types) pass.
test_TypecheckCompileLetsDataTypes :: TestTree
test_TypecheckCompileLetsDataTypes =
ignoreTest $ testProperty "typechecking" $
Copy link
Contributor

Choose a reason for hiding this comment

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

So the reason we can't say it's expected to fail is because it sometimes passes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah.

@michaelpj
Copy link
Contributor

Looks like the letrec test also fails sometimes.

@thealmarty
Add tests for more passes.
d491312
@thealmarty
Add expectFail tag.
b42013d
@thealmarty
Add changelog.
f4a9da8
@thealmarty
Tidy up more, address some comments.
1a330b7
@thealmarty
void doesn't compile on 9.6.
a2cdf59
@thealmarty
Add floatTerm test.
c3a3b31
@thealmarty
Add compileLets tests.
bd4cc41
@thealmarty
Fix Lower.
f70a82f
@thealmarty
Only typecheck after the pass.
e0ed12c
@thealmarty
wip organize tests to eash pass's Test.hs.
b468472
@thealmarty
Add deadcode pass test.
74eadff
@thealmarty
Add rename and recSplit tests.
85ab007
@thealmarty
Avoid tasty discover picking up the property tests.
7775749
@thealmarty
Add caseReduce test.
18a6285
@thealmarty
Fix CI.
e0d4935
@thealmarty
Address comment.
ac9ccd5
@thealmarty
Mark deadcode v2 as expected failure.
5a96a9f
@thealmarty
Make the test consistently fail to pass CI.
67fb027
@thealmarty
Add more tests.
4cbc103
@thealmarty
Try even more on deadcode test so that it fails consistently.
2923dca
@thealmarty
Switch to camel case.
9eebd12
@thealmarty
Run runQuoteT stuff together.
6fd7048
@thealmarty
Fix some tests.
1631bc1
@thealmarty
Let tasty-discover detect prop instead and have quickcheck generate a…
0274a60 
…rgs.
@thealmarty
Add thunkRecursions test.
0a45301
@thealmarty
Ignore deadcode pass test to make CI pass.
03d0a7e
@thealmarty thealmarty force-pushed the thealmarty/plt-1398-pir-typechecking-tests branch from b56e2cd to 03d0a7e Compare October 12, 2023 18:04
@thealmarty thealmarty merged commit a14976d into master Oct 12, 2023
7 checks passed
@thealmarty thealmarty deleted the thealmarty/plt-1398-pir-typechecking-tests branch October 12, 2023 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@effectfully effectfully effectfully left review comments

@michaelpj michaelpj michaelpj approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thealmarty @michaelpj @effectfully