Closed JaroslavTulach closed 5 months ago
@radeusgd I think this could be the fix, right?
enso$ git diff
diff --git distribution/lib/Standard/Test/0.0.0-dev/src/Test.enso distribution/lib/Standard/Test/0.0.0-dev/src/Test.enso
index bdde72228..c0b4cd8cd 100644
--- distribution/lib/Standard/Test/0.0.0-dev/src/Test.enso
+++ distribution/lib/Standard/Test/0.0.0-dev/src/Test.enso
@@ -128,7 +128,7 @@ type Test
example_fail = Test.fail "Something went wrong."
fail : Text -> Nothing|Text -> Test_Result
fail message details=Nothing =
- clue_data = State.get Clue
+ clue_data = Panic.catch State.get Clue (_ -> Nothing)
message_with_clue = case clue_data of
Clue.Value add_clue -> add_clue message
_ -> message
I think we would be better to align the State.get
function a little. We generally have get
taking an if_missing
value and at
returning an error.
We should then use State.get Clue Nothing
for the message with clue.
My take would be - disallow calling stuff like should_be_a
outside of Test.specify
- i.e. I would catch this Panic but just to replace it with another - Illegal_State.Error "Test method called outside of Test.specify block."
.
I think I'm fine with having them outside though - but keep in mind we still souldn't use them like that.
For example, if I do:
Test.group "X" <|
a . should_equal b
Test.specify "Testing a-b" <|
...
then if a != b
this will throw a Panic, not fail a test - because what does it mean - it is called within a group but outside of any particular test that it could mark as failed.
That's why I think this kind of usage is wrong and actually should be disallowed even if a == b
- since we cannot check it statically, we can just check this invariant at runtime.
But I'm not strongly attached to this opinion and I think the fix that @jdunkerley is proposing is also OK.
My use-case is: debugging test _Spec
files is problematic as they are too huge. I want to copy out the piece of the test that is failing and run/debug just that one. Artificially disallowing .should_be_a
and other checks to not work in main
function isn't going to make such "debug as little as possible" use-case easy to achieve.
I think we should make all these function work outside of a test and not panic/error so we can indeed easily run subsets.
Lets apply the change so that the should_
function don't Panic.
Radosław Waśko reports a new STANDUP for yesterday (2024-01-16):
Progress: Ensure we can land the fix for missing State for Clue in Test assertions, get the PR ready. Investigate a bug #8777. Start work on #8723 - adding tests for basic Enso_File directory functionality needed for the Secrets tests. Adding missing features: create_directory
. It should be finished by 2024-01-17.
Next Day: Next day I will be working on the #8723 task. Continue adding tests for Enso_File and secrets within directory structure, documenting what works and what does not.
If I use
should_be_a
outside of test method, I get following error:I guess this is unfortunate. If a
Clue
is missing (because there is no test method around), we should still get reasonable report from theshould_be_a
check, not this internal error.Simplest Reproducer