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

Arrow function tests: simplify and improve the tests #3215

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Feb 13, 2021

Arrow function tests: add verification of the arrow retokenization

Arrow function tests: remove lots of code duplication

  • Simplify the arrow function testing by using a helper function to check the token positions.
  • Fix incorrect parameter order of the $expected vs $result parameters in assertSame().
    While when the test passes, this makes no difference, it does make a difference in the error output when a test fails.

Arrow function tests: some simplifications in the rest of the tests

This addresses the same issues as in the previous commit for those functions which need custom "error" messages or have a shared scope closer, resulting in the opener for the scope_closer pointing to another construct

  • Remove duplicate calculations of the scope opener/closer positions.
  • Fix incorrect parameter order of the $expected vs $result parameters in assertSame().
    While when the test passes, this makes no difference, it does make a difference in the error output when a test fails.

Arrow function tests: minor documentation fix

* Simplify the arrow function testing by using a helper function to check the token positions.
* Fix incorrect parameter order of the `$expected` vs `$result` parameters in `assertSame()`.
    While when the test passes, this makes no difference, it does make a difference in the error output when a test fails.
This addresses the same issues as in the previous commit for those functions which need custom "error" messages or have a shared scope closer, resulting in the `opener` for the `scope_closer` pointing to another construct

* Remove duplicate calculations of the scope opener/closer positions.
* Fix incorrect parameter order of the `$expected` vs `$result` parameters in `assertSame()`.
    While when the test passes, this makes no difference, it does make a difference in the error output when a test fails.
@gsherwood gsherwood added this to the 3.6.0 milestone Feb 14, 2021
@gsherwood gsherwood merged commit 9469e30 into squizlabs:master Feb 14, 2021
@gsherwood
Copy link
Member

That looks a lot cleaner :) Thanks

@jrfnl jrfnl deleted the feature/tests-arrow-function-backfill-improve-tests branch February 14, 2021 22:43
@jrfnl
Copy link
Contributor Author

jrfnl commented Feb 14, 2021

@gsherwood We could now even iterate on it further and use a dataprovider for the majority of tests, if you like.

jrfnl added a commit to jrfnl/PHP_CodeSniffer that referenced this pull request Feb 20, 2021
Follow-up on squizlabs#3215 which made these redundant, but didn't remove them.
gsherwood pushed a commit that referenced this pull request Feb 22, 2021
Follow-up on #3215 which made these redundant, but didn't remove them.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants