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

python312Packages.pprintpp: drop nose dependency #330705

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

Sigmanificient
Copy link
Member

Description of changes

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

There’s a fork, https://github.com/joaonc/pprintpp2, that contains a port to pytest. It looks like upstream was interested in merging its changes, but it never happened. Maybe we should switch to that fork, or at least source the pytest patch from 9c65ae8e89ddb71d58153b1531dc969a5948d244. I see that you have some tests seemingly working that are skipped in that patch, although I also wonder if the parameterized tests are actually getting run correctly in this version? wolever/parameterized#167 suggests that the library they use might not work with current pytest versions.

I need to sleep soon so I’ll dig into this further another day unless you beat me to it.

@Sigmanificient
Copy link
Member Author

Sigmanificient commented Jul 29, 2024

I saw the fork, but wasn't sure about switching to it like this
We currently use PyPI for source, and the patch isn't working on it (i guess some files are missing?)

can't find file to patch at input line 3
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|--- a/.github/workflows/tests.yml
|+++ b/.github/workflows/tests.yml
--------------------------
File to patch: 
Skip this patch? [y] 
Skipping patch.
1 out of 1 hunk ignored
patching file .gitignore
patching file CHANGELOG.txt
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file CHANGELOG.txt.rej
patching file test-requires.txt
patching file test.py

@emilazy
Copy link
Member

emilazy commented Jul 29, 2024

You can use excludes to skip those files, although I think that we should certainly source this from GitHub, as there are patches even beyond the last release (0.4.0, which this is behind).

I think switching to the fork and giving it a 0.4.0-unstable-YYYY-MM-DD version is the best option, given that the upstream maintainer showed intent to merge its changes but then never got around to it; that’s approval enough for me. We could drop our existing patches and this new one, although it might be preferable to keep some of your changes if they allow running tests that are disabled in the fork.

@Sigmanificient
Copy link
Member Author

Let's do that then!

Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

The check inputs disappeared?

@Sigmanificient
Copy link
Member Author

Sigmanificient commented Jul 29, 2024

Seems like the fork has the same issue has me: 2 passed, 3 deselected, 3 xfailed, 3 warnings in 0.24s

@emilazy
Copy link
Member

emilazy commented Jul 29, 2024

That seems okay? xfailed tests are meant to fail. Can you share the build log?

@Sigmanificient
Copy link
Member Author

That seems okay? xfailed tests are meant to fail. Can you share the build log?

Woops I think the important part is this:

=============================== warnings summary ===============================
../../nix/store/xkdqvv523lvzaxkqy4rn1y60yflax19m-python3.11-nose-1.3.7/lib/python3.11/site-packages/nose/plugins/manager.py:418
  /nix/store/xkdqvv523lvzaxkqy4rn1y60yflax19m-python3.11-nose-1.3.7/lib/python3.11/site-packages/nose/plugins/manager.py:418: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
    import pkg_resources

../../nix/store/xkdqvv523lvzaxkqy4rn1y60yflax19m-python3.11-nose-1.3.7/lib/python3.11/site-packages/nose/importer.py:12
  /nix/store/xkdqvv523lvzaxkqy4rn1y60yflax19m-python3.11-nose-1.3.7/lib/python3.11/site-packages/nose/importer.py:12: DeprecationWarning: the imp module is deprecated in favour of importlib and slated for removal in Python 3.12; see the module's documentation for alternative uses
    from imp import find_module, load_module, acquire_lock, release_lock

test.py:81
  test.py:81: PytestCollectionWarning: yield tests were removed in pytest 4.0 - test_unicode will be ignored
    @parameterized([

test.py:92
  test.py:92: PytestCollectionWarning: yield tests were removed in pytest 4.0 - test_back_and_forth will be ignored
    @parameterized([

test.py:139
  test.py:139: PytestCollectionWarning: yield tests were removed in pytest 4.0 - test_expected_input will be ignored
    @parameterized([

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

@emilazy
Copy link
Member

emilazy commented Jul 29, 2024

That looks like you’re building the wrong version of the package. It’s still using nose and joaonc/pprintpp2@9c65ae8 should have removed the use of the parameterized library.

@Sigmanificient
Copy link
Member Author

Oh hum, no sure what happened

@emilazy
Copy link
Member

emilazy commented Jul 31, 2024

Can you put the testing stuff back? I don’t see any reason it shouldn’t work.

@Sigmanificient
Copy link
Member Author

Can you put the testing stuff back? I don’t see any reason it shouldn’t work.

I am not sure what you meant with this one

Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

Uh, sorry, I completely missed the nativeCheckInputs = [ pytestCheckHook ] line last time I looked at the diff since it moved lower down in the file…

Result of nixpkgs-review pr 330705 run on aarch64-linux 1

4 packages marked as broken and skipped:
  • python311Packages.ward
  • python311Packages.ward.dist
  • python312Packages.ward
  • python312Packages.ward.dist
26 packages built:
  • opshin
  • opshin.dist
  • python311Packages.audio-metadata
  • python311Packages.audio-metadata.dist
  • python311Packages.pluthon
  • python311Packages.pluthon.dist
  • python311Packages.pprintpp
  • python311Packages.pprintpp.dist
  • python311Packages.pycardano
  • python311Packages.pycardano.dist
  • python311Packages.tbm-utils
  • python311Packages.tbm-utils.dist
  • python311Packages.uplc
  • python311Packages.uplc.dist
  • python312Packages.audio-metadata
  • python312Packages.audio-metadata.dist
  • python312Packages.pluthon
  • python312Packages.pluthon.dist
  • python312Packages.pprintpp
  • python312Packages.pprintpp.dist
  • python312Packages.pycardano
  • python312Packages.pycardano.dist
  • python312Packages.tbm-utils
  • python312Packages.tbm-utils.dist
  • python312Packages.uplc
  • python312Packages.uplc.dist

@Sigmanificient
Copy link
Member Author

Sigmanificient commented Jul 31, 2024

ward already broken:

    # https://github.com/darrenburns/ward/issues/380
    broken = versionAtLeast rich.version "13.0.0";

@emilazy
Copy link
Member

emilazy commented Jul 31, 2024

An awful lot of tests are still being skipped but that’s for the upstreams to work out.

@emilazy emilazy merged commit 241af39 into NixOS:master Jul 31, 2024
26 of 27 checks passed
@Sigmanificient
Copy link
Member Author

An awful lot of tests are still being skipped but that’s for the upstreams to work out.

Yeah :<

@Sigmanificient Sigmanificient deleted the pprintpp branch July 31, 2024 18:52
@emilazy
Copy link
Member

emilazy commented Jul 31, 2024

Was ward already broken?

Yes, hence “marked broken” (broken = versionAtLeast rich.version "13.0.0";). It seems to be another broken test framework, ironically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants