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
pythonPackages.m3u8: init at 0.5.2 #74110
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no tests located in the pypi package, do you mind checking out the code from github:
running build_ext
----------------------------------------------------------------------
Ran 0 tests in 0.000s
OK
Finished executing setuptoolsCheckPhase
/nix/store/93asqnc8d8q1bpw32dfc198xfn9wxx2s-python3.7-m3u8-0.5.2
the python test are done running runtests and it sets up a http server, is that what we want in a python test? |
There's some tests such as https://github.com/globocom/m3u8/blob/master/tests/test_parser.py which seem to be unit tests. It doesn't have to be full integration tests, but tests are useful to ensure that the application still works in most use cases. |
b86ce66
to
d75076a
Compare
@jonringer added the tests |
${python.interpreter} -m pytest tests/test_parser.py | ||
${python.interpreter} -m pytest tests/test_model.py | ||
${python.interpreter} -m pytest tests/test_variant_m3u8.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be simply:
pytest tests/test_{parser,model,variant_m3u8}.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytest can accept multiple paths, and {...} is bash expansion
d75076a
to
459cff6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytest tests/test_{parser,model,variant_m3u8}.py
runs the same tests as
pytest tests/test_parser.py
pytest tests/test_model.py
pytest tests/test_variant_m3u8.py
but it collects them all into a single run (which is easier to digest)
6300208
to
da2a954
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
diff LGTM
commits LGTM
has tests 👍
[5 built, 54 copied (21.9 MiB), 4.1 MiB DL]
https://github.com/NixOS/nixpkgs/pull/74110
3 package were built:
python27Packages.m3u8 python37Packages.m3u8 python38Packages.m3u8
@GrahamcOfBorg build python27Packages.m3u8 python37Packages.m3u8 python38Packages.m3u8 |
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @