Skip to content

Conversation

TPGamesNL
Copy link
Member

@TPGamesNL TPGamesNL commented Dec 12, 2020

Description

Fix issues with ExprDifference.

Test script used: https://pastebin.com/BhZKtvMr


Target Minecraft Versions: any
Requirements: none
Related Issues: #661, #575, #994, #1001, #2302

@ShaneBeee
Copy link
Contributor

Maybe it would be a good idea to add a regression test for this, to make sure all cases work and this doesn't break anything else.

@TPGamesNL
Copy link
Member Author

TPGamesNL commented Jan 14, 2021

So I tried some tests:

test "difference expression with parse expression":
	set {_a} to 3
	set {_b} to 5
	assert difference between {_a} and {_b} is 2 with "difference expr with a=var and b=var failed"
	assert difference between {_a} and "%{_b}%" parsed as a number is 2 with "difference expr with a=var and b=parse failed"
	assert difference between "%{_a}%" parsed as a number and {_b} is 2 with "difference expr with a=parse and b=var failed"
	assert difference between "%{_a}%" parsed as a number and "%{_b}%" parsed as a number is 2 with "difference expr with a=parse and b=parse failed"

and, because I wasn't running it on my fix, got a console error: https://pastebin.com/bH77tMnc. The problem is, after it threw this error, testing stopped completely. I suggest catching exceptions in some places where the tests can lead to exceptions.

EDIT: I'd also like to draw your attention to https://github.com/SkriptLang/Skript/runs/1701328945?check_suite_focus=true#step:7:144, not sure if it's been reported or not

@TPGamesNL TPGamesNL added 2.5 bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. labels Feb 19, 2021
APickledWalrus
APickledWalrus previously approved these changes Mar 1, 2021
@Whimsyturtle Whimsyturtle changed the base branch from dev-2.5 to master March 23, 2021 12:52
@Whimsyturtle Whimsyturtle dismissed APickledWalrus’s stale review March 23, 2021 12:52

The base branch was changed.

@Whimsyturtle Whimsyturtle merged commit 45b348e into SkriptLang:master Mar 23, 2021
@TPGamesNL TPGamesNL deleted the fix/difference-expr branch March 23, 2021 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants