-
Notifications
You must be signed in to change notification settings - Fork 326
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
Promote broken values instead of ignoring them #11777
Merged
Merged
Changes from 21 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
1c2d007
Currently a DataflowError may get lost in a middle of block statements
JaroslavTulach 49c628e
Propagate Error ASAP instead of ignoring it
JaroslavTulach 91f7497
check for dataflow error in should_equal rhs and report it - this is …
radeusgd 1ed8505
add error check in remaining should_equal overrides
radeusgd 7373304
align behaviour of find across range/vector
radeusgd 00516f5
Merge remote-tracking branch 'origin/develop' into wip/jtulach/Propag…
JaroslavTulach b0f71f8
Fix Decimal test and `Array_Like_Helpers.find` (#11883)
GregoryTravis ee7c81e
Merge remote-tracking branch 'origin/develop' into wip/jtulach/Propag…
JaroslavTulach d703496
Space before dot
JaroslavTulach 7901a25
fix mistake in should_equal check
radeusgd ba16c47
fix frame offsets
radeusgd 6e0928a
fixing more offsets
radeusgd bed6609
fix invalid tests
radeusgd 1647c53
remove unnecessary Not_Found type (clashing with another existing one)
radeusgd ef80339
remove check that was actually too strict
radeusgd a8571c4
fix cloud test cleanup
radeusgd 6551756
fix a test which was propagating error and crashing suite builder
radeusgd 09fb17f
fixing other tests that break Table Tests progress
radeusgd 3b8c089
fixing more issues in Table tests
radeusgd 434ba8a
enable Context to fix lazy init issue
radeusgd 777c38b
Merging with latest develop
JaroslavTulach 8748349
Speculate on statements returning Nothing
JaroslavTulach b7430cb
Code style issues found in CHANGELOG.md file
JaroslavTulach fe5b9e1
duplicate rhs_error_check and make it imperative to avoid nesting
radeusgd 3ce19c7
fix offsets and finally add tests for this
radeusgd afedc71
Documenting propagation of _broken values_
JaroslavTulach bda76cf
Renaming to Errors & Panics
JaroslavTulach 0130601
Merge remote-tracking branch 'origin/develop' into wip/jtulach/Propag…
JaroslavTulach 5916204
Reorganizing changelog
JaroslavTulach 5bfe082
Merge branch 'develop' into wip/jtulach/PropagateDataflowError5430
mergify[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
from Standard.Base import all | ||
import Standard.Base.Errors.Common.Incomparable_Values | ||
import Standard.Base.Errors.Common.No_Such_Method | ||
import Standard.Base.Errors.Illegal_Argument.Illegal_Argument | ||
|
||
import project.Spec_Result.Spec_Result | ||
import project.Test.Test | ||
from project.Extensions_Helpers import rhs_error_check | ||
|
||
## Expect a function to fail with the provided dataflow error. | ||
|
||
|
@@ -70,23 +72,24 @@ Error.should_fail_with self matcher frames_to_skip=0 unwrap_errors=True = | |
|
||
example_should_equal = Examples.add_1_to 1 . should_equal 2 | ||
Any.should_equal : Any -> Integer -> Spec_Result | ||
Any.should_equal self that frames_to_skip=0 = case self == that of | ||
True -> Spec_Result.Success | ||
False -> | ||
loc = Meta.get_source_location 2+frames_to_skip | ||
additional_comment = case self of | ||
_ : Vector -> case that of | ||
_ : Vector -> | ||
case self.length == that.length of | ||
True -> | ||
diff = self.zip that . index_of p-> | ||
p.first != p.second | ||
"; first difference at index " + diff.to_text + " " | ||
False -> "; lengths differ (" + self.length.to_text + " != " + that.length.to_text + ") " | ||
Any.should_equal self that frames_to_skip=0 = rhs_error_check that <| | ||
loc = Meta.get_source_location 4+frames_to_skip | ||
case self == that of | ||
True -> Spec_Result.Success | ||
False -> | ||
additional_comment = case self of | ||
_ : Vector -> case that of | ||
_ : Vector -> | ||
case self.length == that.length of | ||
True -> | ||
diff = self.zip that . index_of p-> | ||
p.first != p.second | ||
"; first difference at index " + diff.to_text + " " | ||
False -> "; lengths differ (" + self.length.to_text + " != " + that.length.to_text + ") " | ||
_ -> "" | ||
_ -> "" | ||
_ -> "" | ||
msg = self.pretty + " did not equal " + that.pretty + additional_comment + " (at " + loc + ")." | ||
Test.fail msg | ||
msg = self.pretty + " did not equal " + that.pretty + additional_comment + " (at " + loc + ")." | ||
Test.fail msg | ||
|
||
## Asserts that `self` value is equal to the expected type value. | ||
|
||
|
@@ -130,12 +133,13 @@ Error.should_equal_type self that frames_to_skip=0 = | |
|
||
example_should_not_equal = Examples.add_1_to 1 . should_not_equal 2 | ||
Any.should_not_equal : Any -> Integer -> Spec_Result | ||
Any.should_not_equal self that frames_to_skip=0 = case self != that of | ||
True -> Spec_Result.Success | ||
False -> | ||
loc = Meta.get_source_location 2+frames_to_skip | ||
msg = self.to_text + " did equal " + that.to_text + " (at " + loc + ")." | ||
Test.fail msg | ||
Any.should_not_equal self that frames_to_skip=0 = if that.is_error then (Panic.throw (Illegal_Argument.Error "Expected value provided as `that` for `should_not_equal` cannot be an error, but got: "+that.to_display_text)) else | ||
loc = Meta.get_source_location 2+frames_to_skip | ||
case self != that of | ||
True -> Spec_Result.Success | ||
False -> | ||
msg = self.to_text + " did equal " + that.to_text + " (at " + loc + ")." | ||
Test.fail msg | ||
|
||
## Added so that dataflow errors are not silently lost. | ||
Error.should_not_equal self that frames_to_skip=0 = | ||
|
@@ -183,15 +187,15 @@ Error.should_not_equal_type self that frames_to_skip=0 = | |
|
||
example_should_start_with = "Hello World!" . should_start_with "Hello" | ||
Any.should_start_with : Text -> Integer -> Spec_Result | ||
Any.should_start_with self that frames_to_skip=0 = case self of | ||
_ : Text -> if self.starts_with that then Spec_Result.Success else | ||
loc = Meta.get_source_location 3+frames_to_skip | ||
msg = self.to_text + " does not start with " + that.to_text + " (at " + loc + ")." | ||
Test.fail msg | ||
_ -> | ||
loc = Meta.get_source_location 2+frames_to_skip | ||
msg = self.to_text + " is not a `Text` value (at " + loc + ")." | ||
Test.fail msg | ||
Any.should_start_with self that frames_to_skip=0 = | ||
loc = Meta.get_source_location 1+frames_to_skip | ||
rhs_error_check that <| case self of | ||
_ : Text -> if self.starts_with that then Spec_Result.Success else | ||
msg = self.to_text + " does not start with " + that.to_text + " (at " + loc + ")." | ||
Test.fail msg | ||
_ -> | ||
msg = self.to_text + " is not a `Text` value (at " + loc + ")." | ||
Test.fail msg | ||
|
||
## Asserts that `self` value is a Text value and ends with `that`. | ||
|
||
|
@@ -207,15 +211,15 @@ Any.should_start_with self that frames_to_skip=0 = case self of | |
|
||
example_should_end_with = "Hello World!" . should_end_with "ld!" | ||
Any.should_end_with : Text -> Integer -> Spec_Result | ||
Any.should_end_with self that frames_to_skip=0 = case self of | ||
_ : Text -> if self.ends_with that then Spec_Result.Success else | ||
loc = Meta.get_source_location 3+frames_to_skip | ||
msg = self.to_text + " does not end with " + that.to_text + " (at " + loc + ")." | ||
Test.fail msg | ||
_ -> | ||
loc = Meta.get_source_location 2+frames_to_skip | ||
msg = self.to_text + " is not a `Text` value (at " + loc + ")." | ||
Test.fail msg | ||
Any.should_end_with self that frames_to_skip=0 = | ||
loc = Meta.get_source_location 1+frames_to_skip | ||
rhs_error_check that <| case self of | ||
_ : Text -> if self.ends_with that then Spec_Result.Success else | ||
msg = self.to_text + " does not end with " + that.to_text + " (at " + loc + ")." | ||
Test.fail msg | ||
_ -> | ||
msg = self.to_text + " is not a `Text` value (at " + loc + ")." | ||
Test.fail msg | ||
|
||
## Asserts that `self` value is a Text value and starts with `that`. | ||
|
||
|
@@ -266,9 +270,8 @@ Error.should_end_with self that frames_to_skip=0 = | |
|
||
example_should_equal = Examples.add_1_to 1 . should_equal 2 | ||
Error.should_equal : Any -> Integer -> Spec_Result | ||
Error.should_equal self that frames_to_skip=0 = | ||
_ = [that] | ||
Test.fail_match_on_unexpected_error self 1+frames_to_skip | ||
Error.should_equal self that frames_to_skip=0 = rhs_error_check that <| | ||
Test.fail_match_on_unexpected_error self 4+frames_to_skip | ||
|
||
## Asserts that `self` is within `epsilon` from `that`. | ||
|
||
|
@@ -294,15 +297,20 @@ Error.should_equal self that frames_to_skip=0 = | |
1.00000001 . should_equal 1.00000002 epsilon=0.0001 | ||
Number.should_equal : Float -> Float -> Integer -> Spec_Result | ||
Number.should_equal self that epsilon=0 frames_to_skip=0 = | ||
matches = case that of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is way easier of parse, we the function signature clearly stays unaffected. |
||
n : Number -> self.equals n epsilon | ||
_ -> self==that | ||
case matches of | ||
True -> Spec_Result.Success | ||
False -> | ||
loc = Meta.get_source_location 2+frames_to_skip | ||
msg = self.to_text + " did not equal " + that.to_text + " (at " + loc + ")." | ||
Test.fail msg | ||
loc = Meta.get_source_location 1+frames_to_skip | ||
rhs_error_check that <| | ||
matches = case that of | ||
n : Number -> self.equals n epsilon . catch Incomparable_Values _-> | ||
## Incomparable_Values is thrown if one of the values is NaN. | ||
We fallback to is_same_object_as, | ||
because in tests we actually NaN.should_equal NaN to succeed. | ||
self.is_same_object_as n | ||
_ -> self==that | ||
case matches of | ||
True -> Spec_Result.Success | ||
False -> | ||
msg = self.to_text + " did not equal " + that.to_text + " (at " + loc + ")." | ||
Test.fail msg | ||
|
||
## Asserts that `self` is within `epsilon` from `that`. | ||
|
||
|
@@ -312,8 +320,8 @@ Number.should_equal self that epsilon=0 frames_to_skip=0 = | |
- frames_to_skip (optional, advanced): used to alter the location which is | ||
displayed as the source of this error. | ||
Decimal.should_equal : Number -> Float-> Float -> Integer -> Spec_Result | ||
Decimal.should_equal self that epsilon=0 frames_to_skip=0 = | ||
self.to_float . should_equal that.to_float epsilon frames_to_skip+1 | ||
Decimal.should_equal self that epsilon=0 frames_to_skip=0 = rhs_error_check that <| | ||
self.to_float . should_equal that.to_float epsilon frames_to_skip+4 | ||
|
||
## Asserts that `self` value is not an error. | ||
|
||
|
@@ -427,7 +435,7 @@ Any.should_be_a self typ = | |
fail_on_wrong_arg_type = | ||
Panic.throw <| | ||
Illegal_Argument.Error "typ ("+typ.to_display_text+") must either be a type or a constructor. Use `should_equal` for value equality test instead." | ||
case Meta.meta typ of | ||
rhs_error_check typ <| case Meta.meta typ of | ||
c : Meta.Constructor -> case Meta.meta self of | ||
a : Meta.Atom -> | ||
if a.constructor == c then Spec_Result.Success else | ||
|
@@ -490,6 +498,8 @@ Any.should_be_a self typ = | |
Any.should_equal_ignoring_order : Any -> Integer -> Spec_Result | ||
Any.should_equal_ignoring_order self that frames_to_skip=0 = | ||
loc = Meta.get_source_location 1+frames_to_skip | ||
if that.is_a Vector . not then | ||
Panic.throw (Illegal_Argument.Error "Expected a Vector, but got a "+that.to_display_text+" (at "+loc+").") | ||
that.each element-> | ||
if self.contains element . not then | ||
msg = "The collection (" + self.to_text + ") did not contain " + element.to_text + " (at " + loc + ")." | ||
|
@@ -557,7 +567,7 @@ Error.should_equal_ignoring_order self that frames_to_skip=0 = | |
Any.should_only_contain_elements_in : Any -> Integer -> Spec_Result | ||
Any.should_only_contain_elements_in self that frames_to_skip=0 = | ||
loc = Meta.get_source_location 1+frames_to_skip | ||
self.each element-> | ||
rhs_error_check that <| self.each element-> | ||
if that.contains element . not then | ||
msg = "The collection contained an element ("+element.to_text+") which was not expected (at " + loc + ")." | ||
Test.fail msg | ||
|
@@ -610,13 +620,14 @@ Error.should_only_contain_elements_in self that frames_to_skip=0 = | |
Any.should_contain : Any -> Integer -> Spec_Result | ||
Any.should_contain self element frames_to_skip=0 = | ||
loc = Meta.get_source_location 1+frames_to_skip | ||
contains_result = Panic.catch No_Such_Method (self.contains element) caught_panic-> | ||
if caught_panic.payload.method_name != "contains" then Panic.throw caught_panic else | ||
msg = "The value (" + self.to_text + ") does not support the method `contains` (at " + loc + ")." | ||
rhs_error_check element <| | ||
contains_result = Panic.catch No_Such_Method (self.contains element) caught_panic-> | ||
if caught_panic.payload.method_name != "contains" then Panic.throw caught_panic else | ||
msg = "The value (" + self.to_text + ") does not support the method `contains` (at " + loc + ")." | ||
Test.fail msg | ||
if contains_result then Spec_Result.Success else | ||
msg = "The value (" + self.to_text + ") did not contain the element (" + element.to_text + ") (at " + loc + ")." | ||
Test.fail msg | ||
if contains_result then Spec_Result.Success else | ||
msg = "The value (" + self.to_text + ") did not contain the element (" + element.to_text + ") (at " + loc + ")." | ||
Test.fail msg | ||
|
||
## Asserts that `self` value contains an element. | ||
|
||
|
@@ -653,13 +664,14 @@ Error.should_contain self element frames_to_skip=0 = | |
Any.should_not_contain : Any -> Integer -> Spec_Result | ||
Any.should_not_contain self element frames_to_skip=0 = | ||
loc = Meta.get_source_location 1+frames_to_skip | ||
contains_result = Panic.catch No_Such_Method (self.contains element) caught_panic-> | ||
if caught_panic.payload.method_name != "contains" then Panic.throw caught_panic else | ||
msg = "The value (" + self.to_text + ") does not support the method `contains` (at " + loc + ")." | ||
rhs_error_check element <| | ||
contains_result = Panic.catch No_Such_Method (self.contains element) caught_panic-> | ||
if caught_panic.payload.method_name != "contains" then Panic.throw caught_panic else | ||
msg = "The value (" + self.to_text + ") does not support the method `contains` (at " + loc + ")." | ||
Test.fail msg | ||
if contains_result.not then Spec_Result.Success else | ||
msg = "The value (" + self.to_text + ") contained the element (" + element.to_text + "), but it was expected to not contain it (at " + loc + ")." | ||
Test.fail msg | ||
if contains_result.not then Spec_Result.Success else | ||
msg = "The value (" + self.to_text + ") contained the element (" + element.to_text + "), but it was expected to not contain it (at " + loc + ")." | ||
Test.fail msg | ||
|
||
## Asserts that `self` value does not contain an element. | ||
|
||
|
14 changes: 14 additions & 0 deletions
14
distribution/lib/Standard/Test/0.0.0-dev/src/Extensions_Helpers.enso
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
from Standard.Base import all | ||
radeusgd marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import Standard.Base.Errors.Illegal_Argument.Illegal_Argument | ||
|
||
## PRIVATE | ||
A helper that ensures that the expected value provided in some of the Test | ||
operations is not an error. | ||
The left-hand side may be an error and that will cause a test failure. | ||
But the right-hand side being an error is bad test design and should be fixed. | ||
rhs_error_check that ~action = | ||
case that.is_error of | ||
False -> action | ||
True -> | ||
msg = "Dataflow error ("+that.to_display_text+") provided as expected value. Use `should_fail_with` or change the test."+ ' Error stack trace was:\n'+that.get_stack_trace_text | ||
Panic.throw (Illegal_Argument.Error msg) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Finally I see it. This wraps the original code in
rhs_error_check
helper function. It doesn't change API ofAny.should_equal
(it is not easy to see that when argument definition is mixed with code on the same line). As a result of the additional function call, the frames shift and thus4+frame_to_skip
. The rest remains unchanged.