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

Remove the noqa and adjust test reference files #982

Merged
merged 1 commit into from
Sep 9, 2023

Conversation

hillairet
Copy link
Contributor

As planned I removed the flake8: noqa from the top of the files that had it.

I was surprised to see that the tests failed till I realized that the change in line number would affect the expected exception strings.
Oh well, a little bit of editing later and it's corrected.
I'd love to know how you make these .txt files with the expected output. I'm happy to document the process too if you want in CONTRIBUTING.
For this time around though the editing may have been more straight forward.

@Delgan
Copy link
Owner

Delgan commented Sep 9, 2023

Apologies, @hillairet, I should have been more proactive and talked to you about this earlier in #955. When modifying the source code for testing exception formatting (or when adding new tests), you can uncomment this line and run the tests to automatically recreate the .txt files:

# generate(stderr, outpath)

Sorry you had to update the files manually. 😨

However, I prefer not to mention this in CONTRIBUTING. This file is a fairly high-level description, whereas the generate() function is an implementation detail related to a very specific test. Instead, I'll try to be more responsive next time to guide contributors properly.

I'm merging this PR. Again, thanks a lot for your help! 😃

@Delgan Delgan merged commit db6c40b into Delgan:master Sep 9, 2023
15 checks passed
@hillairet
Copy link
Contributor Author

Oh one line change? 😅
Well I should have just been more patient and have taken the time to ask you before assuming that it'd be faster to edit. No harm done.

@hillairet hillairet deleted the clean-up-flake8-noqa branch September 9, 2023 22:47
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