Skip to content

Add test README #3014

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

Merged
merged 4 commits into from
Mar 18, 2017
Merged

Add test README #3014

merged 4 commits into from
Mar 18, 2017

Conversation

pkch
Copy link
Contributor

@pkch pkch commented Mar 16, 2017

Add test-data/unit/README.md: move info from wiki, add more stuff

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This will be really helpful.


To add a simple unit test for a new feature you developed, open or create a
`test-data/unit/check-*.test` file with a name that roughly relates to the
feature you added (or create a new file).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you already said "or create" on line 8 above

# indicated file (see Fixtures section for details)
[builtins fixtures/isinstancelist.pyi]

# the optional [out] indicates that any text after it contains the expected
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we say that "E: " is preferred because it makes it easier to associate the errors with the code generating them at a glance, and it makes it easier to change the code of the test without having to change line numbers in [out]?

Also would be helpful to add an example out line.

- The builtins used by default in unit tests live in
`test-data/unit/lib-stub`.

- Individual tests cases can override the stubs by using `[builtins
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test cases

@pkch pkch force-pushed the test-readme branch 2 times, most recently from d7e542a to 2508e0d Compare March 16, 2017 08:15
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me now.

@ilevkivskyi
Copy link
Member

Also it will be very helpful to add some info on how to run tests (for example how to run a single test, or a selection of tests, etc).

any text after it contains the expected type checking error messages.
usually, "E: " is preferred because it makes it easier to associate the
errors with the code generating them at a glance, and to change the code of
the test without having to change line numbers in [out]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put [out] in backquotes here, and also [opt] and [builtins] above.

@ilevkivskyi
Copy link
Member

Also it will be very helpful to add some info on how to run tests (for example how to run a single test, or a selection of tests, etc).

@pkch Ah sorry I didn't notice the last part about running tests with "(this should be expanded)".

@pkch
Copy link
Contributor Author

pkch commented Mar 16, 2017

@ilevkivskyi hmm should I move this section from the main README.md to this one?

@ilevkivskyi
Copy link
Member

@pkch I would prefer to have that section in test README. In the main one it will be overlooked by most people. Plus this will simplify the main README, which is probably too large now.

zzz: str # E: Name 'zzz' already defined

- no code here is executed, just type checked
- `# flags: ` indicates which flags to use for this unit test
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention that this is optional.

with text "abc..."
- note a space after `E:` and `flags:`
- lines without `# E: ` should cause no type check errors
- optional `[builtins]` tells the type checker to use stubs from the indicated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make it a bit more explicit, such as [builtins fixtures/...].

`test-data/unit/lib-stub`.

- Individual test cases can override the stubs by using `[builtins
fixtures/foo.py]`; this targets files in `test-data/unit/fixtures`; feel
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use .pyi extension, not .py.

(`mypy/test/testpythoneval.py`, `test-data/unit/pythoneval.test`). These
type check programs and run them. Unlike the unit tests, these use the
full builtins and library stubs instead of minimal ones. Run them using
`runtests.py testpythoneval`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add mention of the runtests.py -j option and the pytest -n option for running multiple tests in parallel. These can speed up tests a lot, especially if you have many cores.


`runtests.py` by default runs tests in parallel using as many processes as
there are logical cores the `runtests.py` process is allowed to use (on
Windows this information isn't available, so 2 processes are used by
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to work on macOS either, so maybe update the comment to read as "on some platforms this information ...".

may provide the necessary dependencies.

To use the feature, pass e.g. `--txt-report "$(mktemp -d)"`.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra empty lines at the end of the file.

@JukkaL JukkaL merged commit caa68d5 into python:master Mar 18, 2017
@JukkaL
Copy link
Collaborator

JukkaL commented Mar 18, 2017

Thanks! This is going to very useful for new contributors.

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.

4 participants