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

Add simple force-delay tests + clean-up #5849

Merged
merged 3 commits into from
Mar 25, 2024

Conversation

ana-pantilie
Copy link
Contributor

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

Signed-off-by: Ana Pantilie <ana.pantilie95@gmail.com>
Signed-off-by: Ana Pantilie <ana.pantilie95@gmail.com>
Signed-off-by: Ana Pantilie <ana.pantilie95@gmail.com>
@@ -227,6 +227,18 @@ multiApp = runQuote $ do
app = mkIterAppNoAnn lam [mkConstant @Integer () 1, mkConstant @Integer () 2, mkConstant @Integer () 3]
pure app

forceDelayNoApps :: Term Name PLC.DefaultUni PLC.DefaultFun ()
Copy link
Contributor

Choose a reason for hiding this comment

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

btw you can write these with input files that have program syntax in them. That can be easier for simple programs like this! I'm not sure why this file has so many test cases defined by explicitly constructing ASTs...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh thanks for the tip! I can't seem to find any example, though, could you share one?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, weird, there just aren't any for UPLC? Not sure why. But look at e.g. PlutusIR.Transform.Beta.Test

Copy link
Member

Choose a reason for hiding this comment

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

Yeah UPLC tests have always been done this way. The lack of types makes it easy to construct terms in Haskell. I find it at least as easy as writing UPLC by hand.

@ana-pantilie
Copy link
Contributor Author

Forgot to mark this as ready for review.

I tested the simple force-delay cancellation algorithm we had before my previous PR on the tests I've added, and it works. If someone remembers some test for which it failed when we took a look at this, please let me know. My suspicion is that we just misread the complicated nesting of some force-application-lambda-delay when we were looking at those examples related to my previous PR.

I also addressed some of the comments I got on my PRs after they were merged.

@ana-pantilie ana-pantilie changed the title Add simple force-delay tests Add simple force-delay tests + clean-up Mar 24, 2024
@ana-pantilie ana-pantilie marked this pull request as ready for review March 24, 2024 21:31
@ana-pantilie ana-pantilie requested a review from a team March 24, 2024 21:31
@ana-pantilie ana-pantilie merged commit 3811722 into master Mar 25, 2024
5 checks passed
@ana-pantilie ana-pantilie deleted the ana/plt-9687-check-force-delay branch March 25, 2024 07:16
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.

None yet

3 participants