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

Improvements to EffAssert Logging #6542

Merged

Conversation

sovdeeth
Copy link
Member

@sovdeeth sovdeeth commented Apr 8, 2024

Description

Adds the ability to provide expected and received values for an assertion that will be provided in the error message.

assert <.+> [(1:to fail)] with [error] %string%, expected [value] %object%, [and] (received|got) [value] %object%

For assertions using conditions that implement VerboseAssert (and that don't provide custom expected/got values), EffAssert will automatically grab expected/got messages from the condition itself. This will not work reliably for expressions that change between calls, like random expressions or ExprNow, as the assert effect calls getSingle/getAll a second time. For those scenarios, using a variable or a custom expected/got message is recommended.

So far, only CondCompare and CondIsSet implement VerboseAssert, but any condition that has significant use in tests is a valid target for implementation.

Suggestions are extremely welcome for a better interface name. This is the best kenzie and I could come up with in 5 minutes.

Example:

assert 1 is 2 with "failed number comparison" # sends "failed number comparison (Expected a value equal to 2, got 1)."
assert 1 is 2 or 3 with "failed number list comparison" # sends "failed number list comparison (Expected a value equal to 2 or 3, got 1)."

assert name of player is "tony" with "failed name check" # sends "failed name check (Expected a value equal to tony, got Sahvde)."

assert isNaN(difference between NaN value and NaN value) is true with "difference between NaN and NaN is not NaN", expected NaN value, got difference between NaN value and NaN value
# sends "difference between NaN and NaN is not NaN (Expected NaN value, got <number>)."

This PR also adds line numbers to the test error message:

- [ScriptName.sk]
+ [ScriptName.sk, line 24]

Todo:

  • add expected/got value
  • add line numbers
  • clean up changes

Target Minecraft Versions: any
Requirements: none
Related Issues: none

@sovdeeth sovdeeth added enhancement Feature request, an issue about something that could be improved, or a PR improving something. 2.9 Targeting a 2.9.X version release labels Apr 8, 2024
@sovdeeth sovdeeth marked this pull request as ready for review April 8, 2024 18:58
Copy link
Member

@Moderocky Moderocky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very welcome addition.

Copy link
Member

@AyhamAl-Ali AyhamAl-Ali left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quality changes ⚡

@AyhamAl-Ali AyhamAl-Ali added the feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. label Apr 9, 2024
@Moderocky
Copy link
Member

I think I've fixed the conflicts on this but please just triple check it looks how you imagine it ought to!

@sovdeeth
Copy link
Member Author

I think I've fixed the conflicts on this but please just triple check it looks how you imagine it ought to!

looks correct

@Moderocky Moderocky merged commit 772328b into SkriptLang:dev/feature Apr 17, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.9 Targeting a 2.9.X version release enhancement Feature request, an issue about something that could be improved, or a PR improving something. feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants