Skip to content

tests: Format with black #5785

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

Closed
wants to merge 2 commits into from
Closed

tests: Format with black #5785

wants to merge 2 commits into from

Conversation

dlech
Copy link
Contributor

@dlech dlech commented Mar 23, 2020

This adds the python files in the tests/ directory to be formatted with ./tools/codeformat.py.

In a few places # fmt: off/# fmt: on was used where the code had special formatting for readability or where the test was actually testing the specific formatting.

@dlech
Copy link
Contributor Author

dlech commented Mar 23, 2020

I'm guessing that this was intentionally omitted form the recent formatting changes for a reason, but I thought I would try it anyway.

@dpgeorge
Copy link
Member

I'm guessing that this was intentionally omitted form the recent formatting changes for a reason, but I thought I would try it anyway.

Yes, because it was a big change and needed to be thoroughly looked over, precisely because some tests are actually testing the syntax, eg single vs double quote on strings.

I'm happy to make this change, it keeps things consistent, but would want to go through the changes to make sure everything is OK (and CI is failing at the moment...).

@dpgeorge dpgeorge added the tests Relates to tests/ directory in source label Mar 24, 2020
@dlech dlech force-pushed the black-tests branch 2 times, most recently from ab21f40 to e79db79 Compare March 27, 2020 02:47
@dlech
Copy link
Contributor Author

dlech commented Mar 27, 2020

I've updated this to fix the tests (apparently more to fix) and omit tests/basics for now (most of the compiler tests seem to be in that directory, so we can look over those more closely separately later)

@dlech dlech force-pushed the black-tests branch 4 times, most recently from 13d7dbe to dc08bd2 Compare March 28, 2020 03:19
@dpgeorge
Copy link
Member

Instead of excluding specific tests in EXCLUSIONS I'd prefer to use #fmt: off for those tests, eg like the repl ones and cmd-showbc (but the blanket-wide exclusion for tests/basics/*.py can still stay). This is then more consistent with the use of // *FORMAT-OFF* for uncrustify in parts of the C source.

@dlech
Copy link
Contributor Author

dlech commented Mar 28, 2020

like the repl ones

I tried this but it causes errors:

error: cannot format /Users/david/Documents/GitHub/micropython/tests/cmdline/repl_autocomplete.py: Cannot parse: 3:5: impo sys
error: cannot format /Users/david/Documents/GitHub/micropython/tests/cmdline/repl_emacs_keys.py: Cannot parse: 7:0: 
error: cannot format /Users/david/Documents/GitHub/micropython/tests/cmdline/repl_words_move.py: Cannot parse: 4:3: 2341
error: cannot format /Users/david/Documents/GitHub/micropython/tests/cmdline/repl_cont.py: Cannot parse: 30:0: print(x)

So I don't think we don't have a choice on the repl files.

@dlech dlech force-pushed the black-tests branch 4 times, most recently from 32db35a to e9d276e Compare March 28, 2020 19:11
@dlech
Copy link
Contributor Author

dlech commented Mar 28, 2020

I've added an extra commit to fix the one line of code coverage that was lost.

The qemu-arm test is failing on these two lines in tests/float/float2int_intbig.py

print(int(14187745.0))
print("%d" % 14187745.0)

which prints

14187746
14187746

When the 0 after the . is removed, it prints the correct result.

dlech added 2 commits March 28, 2020 15:12
This adds the python files in the tests/ directory to be formatted with
./tools/codeformat.py. basics/ is excluded for now so we aren't changing
too much at once.

In a few places `# fmt: off`/`# fmt: on` was used where the code
had special formatting for readability or where the test was actually
testing the specific formatting.
Since automatically formatting tests with black, we have lost one line
of code coverage. This adds an explicit test to ensure we are testing
the case that is no longer covered implicitly.
@dlech
Copy link
Contributor Author

dlech commented Mar 28, 2020

It looks like the qemu-arm failure is due to #5831, so I've worked around it with # fmt: off for now.

@dlech
Copy link
Contributor Author

dlech commented Mar 28, 2020

floating point issue is addressed in #5832

@dpgeorge
Copy link
Member

Ok, great, this PR looks good now, so merged in 3dc324d and 6110cd3

@dpgeorge dpgeorge closed this Mar 30, 2020
tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Dec 29, 2021
go into safe mode with a useful message if no CIRCUITPY
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Relates to tests/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants