Enable asserts in the tests #4074

JaroslavTulach · 2023-01-24T03:10:04Z

Pull Request Description

Enso unit tests were running without -ea check enabled and as such various invariant checks in Truffle code were not executed. Let's turn the -ea flag on and fix all the code misbehaves.

Checklist

Please include the following checklist in your PR:

The documentation has been updated if necessary.
All code conforms to the
Scala,
Java,
and
Rust
style guides.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed: Enso GUI was tested when built using BOTH
  ./run ide build and ./run ide watch.

…ing the HostObjectDebugWrapper workaround in a hope of getting GraalVM 22.3.1 update soon

…Node, when the context is known to exist

engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Type.java

Akirathan

I don't understand the reason for another assert hack in options in the context. AFAIK, all the option keys should already be present in the context, unless there is something wrong with the initialization. I have attached comments on appropriate places.

engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java

Akirathan · 2023-01-26T11:13:41Z

...ne/runtime/src/main/scala/org/enso/interpreter/instrument/execution/JobExecutionEngine.scala

-    interpreterContext.executionService.getContext.getEnvironment.getOptions
-      .get(RuntimeServerInfo.JOB_PARALLELISM_KEY)
-      .intValue()
+  private val jobParallelism = context.getJobParallelism


JOB_PARALLELISM_KEY is registered in RuntimeServerInstrument. This option descriptor key should be contained in the context. Unless there is something wrong with the initialization of RuntimeServerInstrument?

hubertp

Mostly minor nits

build.sbt

...uments/src/test/java/org/enso/interpreter/test/instrument/AvoidIdInstrumentationTagTest.java

engine/runtime/src/main/java/org/enso/interpreter/node/ExpressionNode.java

engine/runtime/src/main/java/org/enso/interpreter/node/callable/function/BlockNode.java

engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java

engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Type.java

engine/runtime/src/test/java/org/enso/interpreter/test/BigNumberTest.java

engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/SystemProcessTest.scala

…bject

…nso into wip/jtulach/EnableAsserts_183546453

radeusgd

I'm very glad that we are moving towards testing assertions on the CI 😁

hubertp

I don't agree with some of the style changes introduced here but at the same time I'm not going to be blocking this PR only because of that, considering that we didn't have those checks working for a while.

JaroslavTulach added 20 commits January 19, 2023 07:26

Enable asserts when running unit tests

d5dcc00

Standard tags may only be present when there is a source section

11e2e08

SourceSection needs to fit into Source length

64dcaa2

Cannot access EnsoContext when materializeInstrumentableNodes - remov…

3c0fee8

…ing the HostObjectDebugWrapper workaround in a hope of getting GraalVM 22.3.1 update soon

Atom belongs to EnsoLanguage. Fixes MetaobjectTest.

6acf053

When Nothing isNull, it cannot have a meta object. Fixes MetaIsATest.

3cb24d5

Formatting Java sources

8bb039f

Special treatment for Nothing in the MetaObjectTest

4ca3a21

LEAK_CONTEXT has to be invocable

b5a1e9b

Don't panic when no more steps are expected

ddc5277

Keep representing each BlockNode with RootBody tag

a50b8b0

Making BranchResult an interop value helps to remove few asserts

369a1ac

BranchResult can be package private

efa2d70

Returning the chrome inspector support back. Checking it in Statement…

318fff6

…Node, when the context is known to exist

Mitigating asserts in EnsoBigInteger conversions

54467e2

Don't explicitly specify engine

a8b5553

Make sure every EnsoRootNode is associated with EnsoLanguage

7324dff

Use frgaal to copmiler polyglot-api

6e2fb0a

Don't query JOB_PARALLELISM_KEY to avoid AssertionError

c0b78a3

Fix just "one off" mistake

7de4a06

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

JaroslavTulach requested review from radeusgd and Akirathan January 24, 2023 03:10

JaroslavTulach self-assigned this Jan 24, 2023

JaroslavTulach requested review from 4e6 and hubertp as code owners January 24, 2023 03:10

JaroslavTulach added 2 commits January 24, 2023 04:12

Merging with develop to resolve import conflict

c6e08a8

Shield against off source section offsets

5804b28

JaroslavTulach force-pushed the wip/jtulach/EnableAsserts_183546453 branch from 0e6fb07 to 5804b28 Compare January 24, 2023 06:17

Adopt nodes in HashCodeTest and EqualsTest

8a36fdf

JaroslavTulach added CI: Ready to merge This PR is eligible for automatic merge labels Jan 26, 2023

JaroslavTulach mentioned this pull request Jan 26, 2023

Create static wrappers for builtin types #4077

Merged

4 tasks

radeusgd reviewed Jan 26, 2023

View reviewed changes

engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Type.java Outdated Show resolved Hide resolved

Merge branch 'develop' into wip/jtulach/EnableAsserts_183546453

8865555

Akirathan reviewed Jan 26, 2023

View reviewed changes

hubertp reviewed Jan 26, 2023

View reviewed changes

mergify bot and others added 5 commits January 26, 2023 12:54

Merge branch 'develop' into wip/jtulach/EnableAsserts_183546453

faca939

Checking behavior of Nothing isn't influcenced by changes in getMetaO…

1e57210

…bject

Fixing typo

b243ad5

Introducing isNothing check

11710c4

Merge branch 'wip/jtulach/EnableAsserts_183546453' of enso:enso-org/e…

b7a0875

…nso into wip/jtulach/EnableAsserts_183546453

JaroslavTulach requested a review from jdunkerley as a code owner January 26, 2023 15:33

JaroslavTulach requested a review from hubertp January 26, 2023 15:33

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

JaroslavTulach requested a review from radeusgd January 26, 2023 16:07

radeusgd approved these changes Jan 26, 2023

View reviewed changes

Merge branch 'develop' into wip/jtulach/EnableAsserts_183546453

eabcf39

hubertp approved these changes Jan 26, 2023

View reviewed changes

mergify bot merged commit ca2f108 into develop Jan 26, 2023

mergify bot deleted the wip/jtulach/EnableAsserts_183546453 branch January 26, 2023 21:41

This was referenced Feb 6, 2023

Custom eq & hash with consistency #5008

Closed

Invalid Sharing of AST nodes in EpbContext #5087

Closed

Do not instrument lambdas #4898

Closed

Enable asserts when running engine tests #4918

Closed

GraalVM Bump 23.0 to resolve BigInteger issues #5300

Closed

jdunkerley mentioned this pull request Feb 7, 2023

JOB_PARALLELISM option isn't registered when running runtime-with-instruments test #5084

Closed

4 tasks

radeusgd mentioned this pull request Feb 7, 2023

Enable Assertions in Enso Tests #5585

Closed

Akirathan mentioned this pull request Jul 5, 2023

EnsoBigInteger should be a BigInteger polyglot type #7213

Closed

Akirathan mentioned this pull request Sep 25, 2023

Run Enso tests with assertions enabled #7882

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable asserts in the tests #4074

Enable asserts in the tests #4074

JaroslavTulach commented Jan 24, 2023 •

edited

Loading

Akirathan left a comment

Akirathan Jan 26, 2023

hubertp left a comment

radeusgd left a comment

hubertp left a comment

Enable asserts in the tests #4074

Enable asserts in the tests #4074

Conversation

JaroslavTulach commented Jan 24, 2023 • edited Loading

Pull Request Description

Checklist

Akirathan left a comment

Choose a reason for hiding this comment

Akirathan Jan 26, 2023

Choose a reason for hiding this comment

hubertp left a comment

Choose a reason for hiding this comment

radeusgd left a comment

Choose a reason for hiding this comment

hubertp left a comment

Choose a reason for hiding this comment

JaroslavTulach commented Jan 24, 2023 •

edited

Loading