Serde testing infrastructure for the Enso compiler #6062

JaroslavTulach · 2023-03-23T14:14:12Z

Pull Request Description

The primary delivery of this PR is a design of SerdeCompilerTest - a testing suite that allows us to write sample projects, parse them with and without caches and verify they still produce the same IR. This is a similar idea to #3723 which compared the old and new parser IRs.

With infrastructure like this we can start addressing #5567 without any (significant) fear of breaking something essential.

Checklist

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

All code follows the
Scala,
Java,
style guides. In case you are using a language not listed above, follow the Rust style guide.
All code has been tested:
- Unit tests have been written where possible.

…rated

…missing BindingAnalysis

hubertp

I like the approach. Mostly minor nits

engine/runtime/src/main/scala/org/enso/compiler/Compiler.scala

hubertp · 2023-03-26T20:43:17Z

engine/runtime/src/test/java/org/enso/compiler/CompilerTest.java

+    if (!ir1.equals(ir2)) {
+      String name = findTestMethodName();
+      Path home = new File(System.getProperty("user.home")).toPath();
+      Files.writeString(home.resolve(name + ".1"), ir1, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.CREATE, StandardOpenOption.WRITE);


Can we try and not write those to a user's home directory? A directory inside a checked out repo that is simply ignored would be much better

The files are written only in case of failure and are really helpful when one needs to diff them. I was rewriting the old parser to new one without the files being dump and it was plain horrible - e.g. ignoring the files isn't an option. Feel free to suggest better location instead of home. Anyway, this is not code introduced by this PR - I have just refactored it to another class.

Alternatively, it can write to the tmp directory

@JaroslavTulach I know. I didn't like it then, I don't like it now. I understand the sentiment that with hundreds of failures you really want to have something nearby but I also don't like things polluting my home directory.

engine/runtime/src/test/java/org/enso/compiler/CompilerTest.java

engine/runtime/src/test/java/org/enso/compiler/SerdeCompilerTest.java

…ructure

JaroslavTulach · 2023-03-27T08:30:30Z

There were some suspicious test failures probably related not to the Serde testing infrastructure, but the previous changes I did. Removed in 143e4f9 with the hope to narrow the PR down and keep it focus only on the testing infrastructure.

JaroslavTulach · 2023-03-27T09:44:51Z

There is a failure in SerdeCompilerTest which can be reproduced as:

enso$ rm -rf ./target/fake_dir/cache/ir/local/Fib_Test
enso$ sbt runtime/testOnly *SerdeCompilerTest
[error] Test org.enso.compiler.SerdeCompilerTest.testFibTest failed: org.graalvm.polyglot.PolyglotException: Compile error: The name `Arith` could not be found., took 3.903 sec
[error]     at <enso>.Main.main(Main.enso:4)
[error]     at org.graalvm.polyglot.Value.execute(Value.java:878)
[error]     at org.enso.compiler.SerdeCompilerTest.parseSerializedModule(SerdeCompilerTest.java:90)
[error]     at org.enso.compiler.SerdeCompilerTest.testFibTest(SerdeCompilerTest.java:34)
[error]     at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[error]     at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[error]     at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[error]     at java.lang.reflect.Method.invoke(Method.java:566)
[error]     ...
[error] Failed: Total 1, Failed 1, Errors 0, Passed 0
[error] Failed tests:
[error]         org.enso.compiler.SerdeCompilerTest

4e6 · 2023-03-28T06:01:04Z

engine/runtime/src/test/java/org/enso/compiler/CompilerTest.java

+    if (!ir1.equals(ir2)) {
+      String name = findTestMethodName();
+      Path home = new File(System.getProperty("user.home")).toPath();
+      Files.writeString(home.resolve(name + ".1"), ir1, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.CREATE, StandardOpenOption.WRITE);


Alternatively, it can write to the tmp directory

hubertp · 2023-03-28T07:26:25Z

engine/runtime/src/test/java/org/enso/compiler/CompilerTest.java

+    if (!ir1.equals(ir2)) {
+      String name = findTestMethodName();
+      Path home = new File(System.getProperty("user.home")).toPath();
+      Files.writeString(home.resolve(name + ".1"), ir1, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.CREATE, StandardOpenOption.WRITE);


@JaroslavTulach I know. I didn't like it then, I don't like it now. I understand the sentiment that with hundreds of failures you really want to have something nearby but I also don't like things polluting my home directory.

JaroslavTulach · 2023-03-28T07:52:30Z

OK, let's improve our engineering practices a bit. With fc013a7 one gets:

[error] Test org.enso.compiler.SerdeCompilerTest.testFibTest failed: 
Serialized and deserialized IR for Fib_Test expected:
</tmp/testFibTest.1> but was:</tmp/testFibTest.2>, took 2.608 sec
[error] Failed: Total 1, Failed 1, Errors 0, Passed 0

and one can diff the two files then.

JaroslavTulach added 14 commits March 20, 2023 08:13

Test that creates a simple project and makes sure its caches are gene…

de60b35

…rated

{} has no meaning when logging with parameters via Truffle loggers

8f54d29

No need for an impot

74408f3

Collect and display compiler messages when running tests

0a4dba0

Fail on forbidden message

185993f

Avoid need for Module.getIr in ExportsResolution

a509bd9

Instrastructure to capture logs in tests and fail on unwanted messages

bf1bd28

Captures all log messages and prints them when failing

4b0f066

Fails in MethodDefinitions$.runModule(MethodDefinitions.scala:48) on …

f47bba4

…missing BindingAnalysis

Get thru all the import/export passes without parsing the Arith module

44e8d7c

Properly computes 42 by calling Arith

1e36946

Fails with: The name could not be found

2ebc3bc

Cache export imports bindings read from the cache and processed

483207d

SerdeCompilerTest

ed46d51

JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Mar 23, 2023

JaroslavTulach requested a review from 4e6 as a code owner March 23, 2023 14:14

JaroslavTulach self-assigned this Mar 23, 2023

JaroslavTulach requested a review from hubertp as a code owner March 23, 2023 14:14

JaroslavTulach added 5 commits March 23, 2023 15:44

Stable ordering for Metadata.showPassData

8bba511

scalafmtAll

c3633af

Merging with origin/develop

cb07464

Moving simplifyIR to CompilerTest

b717f0f

Reformatted java

09bd913

enso-bot bot mentioned this pull request Mar 24, 2023

Investigate More Effective IR Caching format #5567

Closed

hubertp reviewed Mar 26, 2023

View reviewed changes

Avoiding changes not really necessary for the caching testing infrast…

143e4f9

…ructure

JaroslavTulach added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Mar 27, 2023

Persist all compiledModules for a library

0d983fe

JaroslavTulach removed the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Mar 27, 2023

JaroslavTulach added 2 commits March 27, 2023 15:27

Merge branch 'develop' into wip/jtulach/IR_5567

b424676

Documenting simplifyIR parameters

b0df0cd

JaroslavTulach requested a review from hubertp March 27, 2023 14:26

JaroslavTulach added 2 commits March 27, 2023 16:27

{} has no meaning when logging with parameters via Truffle loggers

ab07a1c

Merge branch 'develop' into wip/jtulach/IR_5567

ee74d14

JaroslavTulach added the CI: Ready to merge This PR is eligible for automatic merge label Mar 28, 2023

Merge branch 'develop' into wip/jtulach/IR_5567

48d0028

4e6 approved these changes Mar 28, 2023

View reviewed changes

hubertp approved these changes Mar 28, 2023

View reviewed changes

Dump the two different IR files into temporary directory

fc013a7

Merge branch 'develop' into wip/jtulach/IR_5567

51bbfd1

mergify bot merged commit 46bd6f6 into develop Mar 28, 2023

mergify bot deleted the wip/jtulach/IR_5567 branch March 28, 2023 08:46

JaroslavTulach mentioned this pull request Mar 28, 2023

Speed Standard.Base initialization in simple Hello World example up! #6100

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Serde testing infrastructure for the Enso compiler #6062

Serde testing infrastructure for the Enso compiler #6062

JaroslavTulach commented Mar 23, 2023 •

edited

Loading

hubertp left a comment

hubertp Mar 26, 2023

JaroslavTulach Mar 27, 2023

4e6 Mar 28, 2023

hubertp Mar 28, 2023 •

edited

Loading

JaroslavTulach commented Mar 27, 2023

JaroslavTulach commented Mar 27, 2023 •

edited

Loading

4e6 Mar 28, 2023

hubertp Mar 28, 2023 •

edited

Loading

JaroslavTulach commented Mar 28, 2023 •

edited

Loading

Serde testing infrastructure for the Enso compiler #6062

Serde testing infrastructure for the Enso compiler #6062

Conversation

JaroslavTulach commented Mar 23, 2023 • edited Loading

Pull Request Description

Checklist

hubertp left a comment

Choose a reason for hiding this comment

hubertp Mar 26, 2023

Choose a reason for hiding this comment

JaroslavTulach Mar 27, 2023

Choose a reason for hiding this comment

4e6 Mar 28, 2023

Choose a reason for hiding this comment

hubertp Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

JaroslavTulach commented Mar 27, 2023

JaroslavTulach commented Mar 27, 2023 • edited Loading

4e6 Mar 28, 2023

Choose a reason for hiding this comment

hubertp Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

JaroslavTulach commented Mar 28, 2023 • edited Loading

JaroslavTulach commented Mar 23, 2023 •

edited

Loading

hubertp Mar 28, 2023 •

edited

Loading

JaroslavTulach commented Mar 27, 2023 •

edited

Loading

hubertp Mar 28, 2023 •

edited

Loading

JaroslavTulach commented Mar 28, 2023 •

edited

Loading