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

refactor the tests #22

Merged
merged 4 commits into from
Jun 9, 2023
Merged

refactor the tests #22

merged 4 commits into from
Jun 9, 2023

Conversation

amtoine
Copy link
Member

@amtoine amtoine commented Jun 9, 2023

cc/ @AucaCoyan

i was having a look at the tests and wanted to give a go at

  • understanding them
  • refactoring them a bit for later ease of maintainance hopefully
    😌

in this PR, i've

  • moved the "ignore" comment to the "reason" of the ignore clause in the ignore_comments test
  • removed whitespaces from handle_escaped_strings because ignore_whitespace_in_string already tests this
  • made the expected part of the asserts explicit
  • put expected always after nu, for consistency

i hope you'll find that somewhat better 🀞

❓ @AucaCoyan, i've got a question 😏
in all the tests, the input string does not have a newline at the end, whereas the output of nufmt does. it is by design?

@AucaCoyan
Copy link
Contributor

in all the tests, the input string does not have a newline at the end, whereas the output of nufmt does. it is by design?

Yes, this is intended. The nufmt should add a new line at the end of the script.

Thanks! It is clearer this way πŸ˜„
You can merge @fdncred 🚒

@fdncred fdncred merged commit b02b7e6 into nushell:main Jun 9, 2023
4 checks passed
@amtoine amtoine deleted the refactor-tests branch June 10, 2023 08:02
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.

None yet

3 participants