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.

Sign up for GitHub

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

Jump to bottom

pythonPackages.pytest-order: init at 0.9.4 #110141

Merged
merged 5 commits into from Jan 28, 2021
Merged

pythonPackages.pytest-order: init at 0.9.4 #110141

merged 5 commits into from Jan 28, 2021

Conversation

jacg
Copy link
Contributor

@jacg jacg commented Jan 20, 2021

Motivation for this change

Package not yet available in nixpkgs

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

dotlambda
dotlambda requested changes Jan 20, 2021
View reviewed changes
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

"Ran 0 tests in 0.000s"

pkgs/development/python-modules/pytest-order/default.nix Outdated Show resolved Hide resolved
@dotlambda
Copy link
Member

Will this be used in some other package you're about to create a pull request for? Then it would be nicer to do a single pull request for all of them.

dotlambda
dotlambda requested changes Jan 20, 2021
View reviewed changes
Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

Commit message and pull request title should start with pythonPackages.

@jacg jacg changed the title pytest-order: init at 0.9.3 pythonPackages.pytest-order: init at 0.9.3 Jan 20, 2021
@jacg
Copy link
Contributor Author

jacg commented Jan 20, 2021

Will this be used in some other package you're about to create a pull request for?

No. It's used in the tests of some projects I've come across, but those are unlikely to be submitted to nixpkgs.

@dotlambda
Copy link
Member

"Ran 0 tests in 0.000s"

You need to run the tests. If they are not distributed on PyPI, you can use fetchFromGitHub.

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 110141 run on x86_64-linux 1

3 packages built:
  • python37Packages.pytest-order
  • python38Packages.pytest-order
  • python39Packages.pytest-order

The following issues got detected with the above build packages.
Please fix at least the ones listed with your changed packages:

python37Packages.pytest-order:

Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
You have the following options to fix this:

  • Tell pytest(CheckHook) where to find the tests included in the package
  • Check if the GitHub Repo contains tests but they are not shipped with Pypi. If so please switch to fetchFromGitHub.
  • If the Packages does not contain any tests add 'doCheck = false;' and a pythonImportsCheck.
python38Packages.pytest-order:

Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
You have the following options to fix this:

  • Tell pytest(CheckHook) where to find the tests included in the package
  • Check if the GitHub Repo contains tests but they are not shipped with Pypi. If so please switch to fetchFromGitHub.
  • If the Packages does not contain any tests add 'doCheck = false;' and a pythonImportsCheck.
python39Packages.pytest-order:

Zero tests run by pytest got detected: 'Ran 0 tests in 0.000s'
You have the following options to fix this:

  • Tell pytest(CheckHook) where to find the tests included in the package
  • Check if the GitHub Repo contains tests but they are not shipped with Pypi. If so please switch to fetchFromGitHub.
  • If the Packages does not contain any tests add 'doCheck = false;' and a pythonImportsCheck.

@jacg
WIP: Run tests by fetching from GitHub rather than PyPI
7800cf7 
Two tests failing, struggling to understand why.
@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 110141 run on x86_64-linux 1

3 packages failed to build and are new build failures:

@jacg
Copy link
Contributor Author

jacg commented Jan 27, 2021

There is apparently one pending requested change here, but I cannot find any that requested changes that are unresolved.

@jacg jacg mentioned this pull request Jan 27, 2021
Closed
@jacg
0.9.3 -> 0.9.4 after tests included in new release
bd9a373 
After discussion with the repo owner, the tests were included in the PyPI
release. This seems to have fixed all problems with execution of the tests in
the fetch-from-github approach.
@jacg jacg changed the title pythonPackages.pytest-order: init at 0.9.3 pythonPackages.pytest-order: init at 0.9.4 Jan 27, 2021
@jacg
Copy link
Contributor Author

jacg commented Jan 27, 2021

After brief discussion with the maintainer of pytest-order, a new release (0.9.4) has been made which includes the tests. I have reverted back to using fetchPypi and the tests now pass.

@jacg @dotlambda
Don't skip checkPhase
843b30d 
Co-authored-by: Robert Schütz <rschuetz17@gmail.com>
@dotlambda dotlambda merged commit 750510e into NixOS:master Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda dotlambda requested changes

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jacg @dotlambda @SuperSandro2000