-
Notifications
You must be signed in to change notification settings - Fork 328
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
Implement Enso-specific assert #7883
Conversation
4380526
to
4d73fba
Compare
Can we ensure that we enable the |
Sure, I added it to the task list so that I don't forget about it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two questions:
- please justify use of
Assertion
for a non-dynamic state. - do we really need new Enso CLI parameter?
...e/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/AssertNode.java
Outdated
Show resolved
Hide resolved
...e/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/AssertNode.java
Show resolved
Hide resolved
...e/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/AssertNode.java
Outdated
Show resolved
Hide resolved
engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java
Outdated
Show resolved
Hide resolved
engine/polyglot-api/src/main/java/org/enso/polyglot/RuntimeOptions.java
Outdated
Show resolved
Hide resolved
This reverts commit 5d4d801.
This reverts commit 882f777.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace InteropLibrary
with EqualsNode
or co. Otherwise OK.
@@ -1334,6 +1335,7 @@ lazy val runtime = (project in file("engine/runtime")) | |||
"org.typelevel" %% "cats-core" % catsVersion, | |||
"junit" % "junit" % junitVersion % Test, | |||
"com.github.sbt" % "junit-interface" % junitIfVersion % Test, | |||
"org.hamcrest" % "hamcrest-all" % hamcrestVersion % Test, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ó, hamcrest... I expect to find some advanced JUnit magic below!
...runtime/src/main/java/org/enso/interpreter/node/expression/builtin/error/AssertionError.java
Outdated
Show resolved
Hide resolved
...e/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/runtime/AssertNode.java
Outdated
Show resolved
Hide resolved
engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java
Show resolved
Hide resolved
Assertions on the CI will be enabled in #7929 |
Closes #5973
Pull Request Description
Implement Enso-specific assert -
Runtime.assert
that works like asserts in any other runtime.Important Notes
ENSO_ENABLE_ASSERTIONS
env var is not empty (Seeenso/engine/runtime/src/main/java/org/enso/interpreter/runtime/EnsoContext.java
Line 139 in 72cd836
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build
.