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

[WIP] Implement IRComparator in tests #12162

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

Akirathan
Copy link
Member

Pull Request Description

All over the compiler tests, we are testing the compiled IR with something like

"desugar nested constructors to simple patterns" in {
ir.isNested shouldBe false
consANilBranch.expression shouldBe an[Expression.Block]
consANilBranch.pattern shouldBe an[Pattern.Constructor]
NestedPatternMatch
.containsNestedPatterns(consANilBranch.pattern) shouldEqual false
consANilBranch.terminalBranch shouldBe false
val nestedCase = consANilBranch.expression
.asInstanceOf[Expression.Block]
.returnValue
.asInstanceOf[Case.Expr]
nestedCase.scrutinee shouldBe an[Name.Literal]
nestedCase.branches.length shouldEqual 1
nestedCase.isNested shouldBe true
val nilBranch = nestedCase.branches(0)
nilBranch.pattern shouldBe a[Pattern.Constructor]
nilBranch.pattern
.asInstanceOf[Pattern.Constructor]
.constructor
.name shouldEqual "Nil"
nilBranch.expression shouldBe an[Name.Literal]
nilBranch.expression.asInstanceOf[Name].name shouldEqual "a"
nilBranch.terminalBranch shouldBe true
}
which seems very cryptic.

When working on #11717 and #12061, it struck me that it could be useful to have a better IR comparison functionality in tests. More specifically, we could provide an expected IR object, compare it to the actual IR, and in case of a failure, we could dump the difference to IGV with #12061. In this PR, I have attempted to provide such functionality.

The problem is that the difference dumped to IGV when there is a test failure on 24638b7 is not useful:
image

In these two graphs, there is actually a single node different - Name.Literal('<internal-10>'), but the diff shows us that the whole graph is different. To make this better and only highlight a single changed node, we need a functionality to replace children. More specifically, we need to duplicate the expected IR, and only replace a single node in the IR. That is, currently, impossible. But it could be possible after all our IR definitions are generated by an annotation processor.

This PR is pushed to the repo, so that this idea is not forgotten.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.
  • If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
    or the Snowflake database integration, a run of the Extra Tests has been scheduled.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

@Akirathan Akirathan self-assigned this Jan 27, 2025
@enso-bot enso-bot bot mentioned this pull request Jan 27, 2025
5 tasks
@Akirathan
Copy link
Member Author

Another idea how to make IR comparisons in tests more readable is in e2e348e - just link a screenshot to an IGV graph, and use variable names with IGV node IDs in tests.

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.

1 participant