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

Enable Assertions in Enso Tests #5585

Closed
1 task
radeusgd opened this issue Feb 7, 2023 · 7 comments · Fixed by #7882
Closed
1 task

Enable Assertions in Enso Tests #5585

radeusgd opened this issue Feb 7, 2023 · 7 comments · Fixed by #7882
Assignees
Labels
-compiler p-medium Should be completed in the next few sprints

Comments

@radeusgd
Copy link
Member

radeusgd commented Feb 7, 2023

How important is this feature to you?

3 – Lack of it makes using Enso slightly harder

Describe the idea you'd like to see implemented.

Recently we finally got assertions enabled in the engine tests. This is great, because now we will be less likely to break Truffle invariants and we can also have our own assertions.

However, engine tests are not everything, a lot of interesting interactions also happen in our 'integration tests' written directly in Enso - in the test/ directory.

Currently, running test/Tests yields:

[info] [2023-02-07T17:03:46.864Z] [enso.org.enso.compiler.PackageRepository$Default] Found library Standard.Base @ 0.0.0-dev at [***\0.0.0-dev].
[info] [2023-02-07T17:03:46.921Z] [enso.org.enso.compiler.PackageRepository$Default] Found library Standard.Test @ 0.0.0-dev at [***\0.0.0-dev].
Exception in thread "Thread-7" java.lang.IllegalStateException: There is no current context available.
        at org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotEngineException.illegalState(PolyglotEngineException.java:135)
        at org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotContextImpl.requireContext(PolyglotContextImpl.java:720)
        at org.graalvm.truffle/com.oracle.truffle.polyglot.EngineAccessor$EngineImpl.createLanguageSystemThread(EngineAccessor.java:1940)      
        at org.graalvm.truffle/com.oracle.truffle.api.TruffleLanguage$Env.createSystemThread(TruffleLanguage.java:1847)
        at org.graalvm.truffle/com.oracle.truffle.api.TruffleLanguage$Env.createSystemThread(TruffleLanguage.java:1813)
        at org.enso.compiler.SerializationManager.$anonfun$pool$1(SerializationManager.scala:54)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.<init>(ThreadPoolExecutor.java:623)
        at java.base/java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:912)
        at java.base/java.util.concurrent.ThreadPoolExecutor.processWorkerExit(ThreadPoolExecutor.java:1005)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
        at java.base/java.lang.Thread.run(Thread.java:829)
        at org.graalvm.truffle/com.oracle.truffle.polyglot.SystemThread.run(SystemThread.java:65)
Execution finished with an error: java.lang.AssertionError: Invalid sharing of AST nodes detected. The current context uses a different sharing layer than the executed node. A common cause of this are CallTargets that are reused across different contexts in an invalid way.Stack trace: 
  <<current-context>>
    <-- Sharing Layer Change: 0x386708CF => 0xB3A8455 -->
  <epb>.(Unknown)(ForeignEvalNode@5c930fc3)
  <epb>.(Unknown)
  <enso>.Array_Polyglot_Spec.data(Array_Polyglot_Spec.enso:68)
  <enso>.Array_Polyglot_Spec.spec<arg-0>(Array_Polyglot_Spec.enso:10)
  <enso>.Array_Polyglot_Spec.spec<arg-1>(Array_Polyglot_Spec.enso:10)
  <enso>.Panic.catch(Unknown)
  <enso>.Panic.type.recover(Panic.enso:156)
  <enso>.Test.expect_panic_with(Test.enso:94)
  <enso>.Array_Polyglot_Spec.spec<arg-1>(Array_Polyglot_Spec.enso:10)
  <enso>.Test.run_spec<arg-1>(Test.enso:190)
  <enso>.Panic.catch(Unknown)
  <enso>.Panic.type.recover(Panic.enso:156)
  <enso>.Function.<|(Unknown)
  <enso>.Test.run_spec(Test.enso:189)
  <enso>.case_branch<arg-3>(Test.enso:70)
  <enso>.State.run(Unknown)
  <enso>.case_branch<arg-1>(Test.enso:70)
  <enso>.Runtime.no_inline(Unknown)
  <enso>.Duration.type.time_execution(Duration.enso:124)
  <enso>.case_branch(Test.enso:70)
  <enso>.Test.specify(Test.enso:69)
  <enso>.Function.<|(Unknown)
  <enso>.Array_Polyglot_Spec.spec<arg-1>(Array_Polyglot_Spec.enso:9)
  <enso>.case_branch<arg-1>(Test.enso:33)
  <enso>.State.run(Unknown)
  <enso>.Function.<|(Unknown)
  <enso>.case_branch(Test.enso:32)
  <enso>.Test.group<arg-1>(Test.enso:30)
  <enso>.Boolean.if_then(Unknown)
  <enso>.Test.group(Test.enso:29)
  <enso>.Function.<|(Unknown)
  <enso>.Array_Polyglot_Spec.spec(Array_Polyglot_Spec.enso:8)
  <enso>.Main.main<arg-1>(Main.enso:85)
  <enso>.Test_Suite.type.run<arg-1>(Test_Suite.enso:69)
  <enso>.State.run(Unknown)
  <enso>.Function.<|(Unknown)
  <enso>.Test_Suite.type.run<arg-1>(Test_Suite.enso:68)
  <enso>.Test_Reporter.wrap_junit_testsuites(Test_Reporter.enso:17)
  <enso>.Function.<|(Unknown)
  <enso>.Test_Suite.type.run(Test_Suite.enso:67)
  <enso>.Test_Suite.type.run_main(Test_Suite.enso:40)
  <enso>.Function.<|(Unknown)
  <enso>.Main.main(Main.enso:81)
  <Unknown>.org.graalvm.polyglot.Value<Function>.execute(Unknown)
        at <java> org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotSharingLayer.invalidSharingError(PolyglotSharingLayer.java:641)      
        at <java> org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotFastThreadLocals.validSharing(PolyglotFastThreadLocals.java:266)     
        at <java> org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotFastThreadLocals.getLanguageContext(PolyglotFastThreadLocals.java:236)
        at <java> org.graalvm.truffle/com.oracle.truffle.polyglot.PolyglotFastThreadLocals$ContextReferenceImpl.get(PolyglotFastThreadLocals.java:457)
        at <java> org.enso.interpreter.epb.EpbContext.get(EpbContext.java:57)
        at <java> org.enso.interpreter.epb.node.ForeignEvalNode.parseJs(ForeignEvalNode.java:118)
        at <java> org.enso.interpreter.epb.node.ForeignEvalNode.lockAndParse(ForeignEvalNode.java:83)
        at <java> org.enso.interpreter.epb.node.ForeignEvalNode.ensureParsed(ForeignEvalNode.java:64)
        at <java> org.enso.interpreter.epb.node.ForeignEvalNode.execute(ForeignEvalNode.java:53)
        at <epb> <epb> null(Unknown)
        at <enso> Array_Polyglot_Spec.data(src\Data\Array_Polyglot_Spec.enso:68-82)
        at <enso> Array_Polyglot_Spec.spec<arg-0>(src\Data\Array_Polyglot_Spec.enso:10:36-39)
        at <enso> Array_Polyglot_Spec.spec<arg-1>(src\Data\Array_Polyglot_Spec.enso:10:36-47)
        at <enso> Panic.catch(Internal)
        at <enso> Panic.type.recover(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\Panic.enso:156-161)
        at <enso> Test.expect_panic_with(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test.enso:94:15-38)
        at <enso> Array_Polyglot_Spec.spec<arg-1>(src\Data\Array_Polyglot_Spec.enso:10:13-68)
        at <enso> Test.run_spec<arg-1>(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test.enso:190:18-25)
        at <enso> Panic.catch(Internal)
        at <enso> Panic.type.recover(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\Panic.enso:156-161)
        at <enso> Function.<|(Internal)
        at <enso> Test.run_spec(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test.enso:189-193)
        at <enso> case_branch<arg-3>(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test.enso:70:73-89)
        at <enso> State.run(Internal)
        at <enso> case_branch<arg-1>(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test.enso:70:49-90)
        at <enso> Runtime.no_inline(Internal)
        at <enso> Duration.type.time_execution(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\Data\Time\Duration.enso:124:18-43)
        at <enso> case_branch(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test.enso:70:24-91)
        at <enso> Test.specify(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test.enso:69-71)
        at <enso> Function.<|(Internal)
        at <enso> Array_Polyglot_Spec.spec<arg-1>(src\Data\Array_Polyglot_Spec.enso:9-10)
        at <enso> case_branch<arg-1>(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test.enso:33:25-33)
        at <enso> State.run(Internal)
        at <enso> Function.<|(Internal)
        at <enso> case_branch(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test.enso:32-34)
        at <enso> Test.group<arg-1>(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test.enso:30-39)
        at <enso> Boolean.if_then(Internal)
        at <enso> Test.group(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test.enso:29-39)
        at <enso> Function.<|(Internal)
        at <enso> Array_Polyglot_Spec.spec(src\Data\Array_Polyglot_Spec.enso:8-64)
        at <enso> Main.main<arg-1>(src\Main.enso:85:5-28)
        at <enso> Test_Suite.type.run<arg-1>(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test_Suite.enso:69:17-21)
        at <enso> State.run(Internal)
        at <enso> Function.<|(Internal)
        at <enso> Test_Suite.type.run<arg-1>(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test_Suite.enso:68-70)
        at <enso> Test_Reporter.wrap_junit_testsuites(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test_Reporter.enso:17:14-19)
        at <enso> Function.<|(Internal)
        at <enso> Test_Suite.type.run(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test_Suite.enso:67-70)
        at <enso> Test_Suite.type.run_main(C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Test\0.0.0-dev\src\Test_Suite.enso:40:13-39)
        at <enso> Function.<|(Internal)
        at <enso> Main.main(src\Main.enso:81-147)

So we are still breaking Truffle assertions.

Moreover, ideally we should be able to use and verify asserts within std-bits. Currently, even if we add these, they cannot really be checked because if we run with -ea the run will fail due to the broken Truffle assertions.

Ideally we should also have -ea enabled for the Enso tests on the CI. Currently, that is only the case for the engine tests.

Problem this is solving

  1. We should not be breaking Truffle invariants checked in the asserts. Apparently, the engine test checks are not enough - we are still breakin these invariants 'in the wild'.
  2. Because of (1), I cannot run Enso test suite with assertions enabled, because it will always fail due to the Truffle invariants being broken. So I cannot verify asserts in std-bits code (because the runs will fail due to (1)). Having the ability to add asserts is helpful as it allows us to verify invariants in our code and write it more correctly. We were also thinking of including an assert construct into Enso directly. Such assertions not only allow us to more confidently verify correctness of our code but also serve as documentation purposes explaining what a given function expects.

Tasks

Preview Give feedback
@Akirathan
Copy link
Member

#5087 is probably a blocker for this issue.

@jdunkerley jdunkerley removed the triage label Feb 10, 2023
@Akirathan Akirathan self-assigned this Jul 7, 2023
@github-project-automation github-project-automation bot moved this to ❓New in Issues Board Jul 7, 2023
@Akirathan Akirathan moved this from ❓New to 📤 Backlog in Issues Board Jul 7, 2023
@Akirathan
Copy link
Member

Bumping the priority - after merging of #7147, an elementary expression like [1] + [2] is now failing on AssertionError with enabled assertions.

@Akirathan Akirathan added the p-medium Should be completed in the next few sprints label Jul 7, 2023
@Akirathan Akirathan moved this from 📤 Backlog to ⚙️ Design in Issues Board Sep 25, 2023
@jdunkerley jdunkerley moved this from ⚙️ Design to 🔧 Implementation in Issues Board Oct 3, 2023
@jdunkerley jdunkerley moved this from 🔧 Implementation to ⚙️ Design in Issues Board Oct 3, 2023
@Akirathan Akirathan moved this from ⚙️ Design to 👁️ Code review in Issues Board Oct 3, 2023
@Akirathan Akirathan moved this from 👁️ Code review to 🔧 Implementation in Issues Board Oct 6, 2023
@Akirathan Akirathan linked a pull request Nov 17, 2023 that will close this issue
2 tasks
@enso-bot
Copy link

enso-bot bot commented Nov 29, 2023

Pavel Marek reports a new STANDUP for today (2023-11-29):

Progress: - Continuing to work on JVM assertions after long time.

  • Looking into the InvalidSharingException in epb language.
  • Playing around with inner contexts.
  • Meetings & reviews It should be finished by 2023-12-06.

@enso-bot
Copy link

enso-bot bot commented Nov 30, 2023

Pavel Marek reports a new STANDUP for today (2023-11-30):

Progress: - VisualVM polyglot sampling does not work?

  • Broken checkout latest develop on Windows - path too long - caused by my recent PR
  • Trying a solution for the InvalidSharingException - create separate root nodes for foreign function calls, instead of inserting them under the current AST.
    • Still running into InvalidSharingException, let's investigate more. It should be finished by 2023-12-06.

@enso-bot
Copy link

enso-bot bot commented Dec 1, 2023

Pavel Marek reports a new STANDUP for today (2023-12-01):

Progress: - Still bumping into multi thread access not allowed for js error.

  • Playing with context and thread listener instruments to see what and when needs to be locked
  • Fixing legal review for graalpy PR a bit. It should be finished by 2023-12-06.

@enso-bot
Copy link

enso-bot bot commented Dec 4, 2023

Pavel Marek reports a new STANDUP for today (2023-12-04):

Progress: - Giving up on this PR for now, published the investigation results as a comment It should be finished by 2023-12-06.

@JaroslavTulach
Copy link
Member

This is done by following line.

@github-project-automation github-project-automation bot moved this from 🔧 Implementation to 🟢 Accepted in Issues Board Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-compiler p-medium Should be completed in the next few sprints
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants