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

lexers: move to comparing bytes in tests #856

Merged
merged 3 commits into from Sep 21, 2023

Conversation

myitcv
Copy link
Contributor

@myitcv myitcv commented Sep 21, 2023

See individual commits for comments on each change, but in summary:

  • Consistently write a newline when RECORD=true
  • Update expected files to have a trailing newline
  • Update lexers/lexers_test.go to compare for byte equality to ensure that the working tree of a commit where the tests pass is left in a clean state when run with RECORD=true

Note that as a result of this change, the following command results in a clean working tree:

RECORD=true go test ./lexers

Fixes #855

This ensures we consistently write a trailing newline when RECORD=true
is set.
This is the mechanical result of running:

    RECORD=true go test ./lexers
This helps to ensure that running RECORD=true go test ./lexers on a
given commit for which the tests pass will not result in any changes in
the working tree.

Notes that as a result of this change, the following command results in
a clean working tree:

    RECORD=true go test ./lexers

Fixes alecthomas#855
@alecthomas
Copy link
Owner

alecthomas commented Sep 21, 2023

I don't think your analysis is correct. As far as I can tell RECORD=true isn't inconsistent with how it adds newlines. My guess is that someone manually updated the files before committing them (or rather their editor probably did it fort them) which added newlines.

(edit)
Your PR will detect that, but I think the mechanics are overkill for such a minor issue. Thanks for the PR anyway.

@alecthomas
Copy link
Owner

.

@alecthomas alecthomas closed this Sep 21, 2023
@alecthomas alecthomas reopened this Sep 21, 2023
@alecthomas
Copy link
Owner

Sorry, early morning brain fart 🤦 , I changed my mind. Let's merge it in

@alecthomas alecthomas merged commit ccd8d68 into alecthomas:master Sep 21, 2023
2 checks passed
@myitcv myitcv deleted the fix_expectations branch September 21, 2023 22:57
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.

Running tests with RECORD=true leaves a non-clean working tree
2 participants