Skip to content

python312Packages.click-option-group: cleanup & skip failing test#391003

Closed
GaetanLepage wants to merge 1 commit intoNixOS:masterfrom
GaetanLepage:click-option-group
Closed

python312Packages.click-option-group: cleanup & skip failing test#391003
GaetanLepage wants to merge 1 commit intoNixOS:masterfrom
GaetanLepage:click-option-group

Conversation

@GaetanLepage
Copy link
Contributor

@GaetanLepage GaetanLepage commented Mar 18, 2025 •
edited
Loading

Things done

=================================== FAILURES ===================================
______________________ test_missing_group_decl_first_api _______________________

runner = <click.testing.CliRunner object at 0x7ffff652ce00>

    def test_missing_group_decl_first_api(runner):
        @click.command()
        @click.option('--hello1')
        @optgroup.option('--foo')
        @optgroup.option('--bar')
        @click.option('--hello2')
        def cli(**params):
            pass

        result = runner.invoke(cli, ['--help'])

>       assert result.exception
E       assert None
E        +  where None = <Result okay>.exception

tests/test_click_option_group.py:111: AssertionError
=========================== short test summary info ============================
FAILED tests/test_click_option_group.py::test_missing_group_decl_first_api - assert None
========================= 1 failed, 30 passed in 0.13s =========================

cc @mweinelt

  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Mar 18, 2025
@GaetanLepage GaetanLepage mentioned this pull request Mar 18, 2025
Merged
@github-actions github-actions bot added 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Mar 18, 2025
@nix-owners nix-owners bot requested a review from mweinelt March 18, 2025 16:09
MattSturgeon
MattSturgeon approved these changes Mar 18, 2025
View reviewed changes
Copy link
Contributor

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

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

I'm not a huge fan of having fixes and cleanup together in the same commit, but the diff looks good overall. Thanks

pkgs/development/python-modules/click-option-group/default.nix
src = fetchFromGitHub {
owner = "click-contrib";
repo = pname;
repo = "click-option-group";
Copy link
Contributor

@MattSturgeon MattSturgeon Mar 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This change seems unnecessary, since the variable happens to be the same.

Re-using pname via rec is not like using finalAttrs.pname, which would change when overridden. Getting the binding via rec is effectively constant.

Copy link
Contributor Author

@GaetanLepage GaetanLepage Mar 18, 2025

Choose a reason for hiding this comment

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

Re-using pname in src.repo is discouraged AFAIK.
Hard-coding it is preferred as it prevents silent failures when the package is renamed.

Copy link
Contributor

@MattSturgeon MattSturgeon Mar 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It is discouraged to inherit it from finalAttrs, since that could cause issues when overriding the pname attr.

I can't see how inheriting via rec would ever lead to silent errors; you would get a 404 and/or hash mismatch if the repo argument became invalid.

Either way, not a big deal.

@GaetanLepage
Copy link
Contributor Author

GaetanLepage commented Mar 18, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 391003

x86_64-linux

⏩ 4 packages marked as broken and skipped:
  • python312Packages.bentoml
  • python312Packages.bentoml.dist
  • python313Packages.bentoml
  • python313Packages.bentoml.dist
❌ 4 packages failed to build:
  • python312Packages.spsdk
  • python312Packages.spsdk.dist
  • python313Packages.spsdk
  • python313Packages.spsdk.dist
✅ 35 packages built:
  • checkov
  • checkov.dist
  • makejinja
  • makejinja.dist
  • mpvScripts.autosub
  • python312Packages.click-option-group
  • python312Packages.click-option-group.dist
  • python312Packages.cloudsplaining
  • python312Packages.cloudsplaining.dist
  • semgrep (python312Packages.semgrep)
  • semgrep.dist (python312Packages.semgrep.dist)
  • python312Packages.subliminal
  • python312Packages.subliminal.dist
  • python312Packages.typed-settings
  • python312Packages.typed-settings.dist
  • whispers (python312Packages.whispers)
  • whispers.dist (python312Packages.whispers.dist)
  • python313Packages.click-option-group
  • python313Packages.click-option-group.dist
  • python313Packages.cloudsplaining
  • python313Packages.cloudsplaining.dist
  • python313Packages.semgrep
  • python313Packages.semgrep.dist
  • python313Packages.subliminal
  • python313Packages.subliminal.dist
  • python313Packages.typed-settings
  • python313Packages.typed-settings.dist
  • python313Packages.whispers
  • python313Packages.whispers.dist
  • route-detect
  • route-detect.dist
  • swaggerhole
  • swaggerhole.dist
  • synadm
  • synadm.dist

aarch64-linux

⏩ 4 packages marked as broken and skipped:
  • python312Packages.bentoml
  • python312Packages.bentoml.dist
  • python313Packages.bentoml
  • python313Packages.bentoml.dist
✅ 35 packages built:
  • checkov
  • checkov.dist
  • makejinja
  • makejinja.dist
  • mpvScripts.autosub
  • python312Packages.click-option-group
  • python312Packages.click-option-group.dist
  • python312Packages.cloudsplaining
  • python312Packages.cloudsplaining.dist
  • semgrep (python312Packages.semgrep)
  • semgrep.dist (python312Packages.semgrep.dist)
  • python312Packages.subliminal
  • python312Packages.subliminal.dist
  • python312Packages.typed-settings
  • python312Packages.typed-settings.dist
  • whispers (python312Packages.whispers)
  • whispers.dist (python312Packages.whispers.dist)
  • python313Packages.click-option-group
  • python313Packages.click-option-group.dist
  • python313Packages.cloudsplaining
  • python313Packages.cloudsplaining.dist
  • python313Packages.semgrep
  • python313Packages.semgrep.dist
  • python313Packages.subliminal
  • python313Packages.subliminal.dist
  • python313Packages.typed-settings
  • python313Packages.typed-settings.dist
  • python313Packages.whispers
  • python313Packages.whispers.dist
  • route-detect
  • route-detect.dist
  • swaggerhole
  • swaggerhole.dist
  • synadm
  • synadm.dist

x86_64-darwin

⏩ 4 packages marked as broken and skipped:
  • python312Packages.bentoml
  • python312Packages.bentoml.dist
  • python313Packages.bentoml
  • python313Packages.bentoml.dist
✅ 35 packages built:
  • checkov
  • checkov.dist
  • makejinja
  • makejinja.dist
  • mpvScripts.autosub
  • python312Packages.click-option-group
  • python312Packages.click-option-group.dist
  • python312Packages.cloudsplaining
  • python312Packages.cloudsplaining.dist
  • semgrep (python312Packages.semgrep)
  • semgrep.dist (python312Packages.semgrep.dist)
  • python312Packages.subliminal
  • python312Packages.subliminal.dist
  • python312Packages.typed-settings
  • python312Packages.typed-settings.dist
  • whispers (python312Packages.whispers)
  • whispers.dist (python312Packages.whispers.dist)
  • python313Packages.click-option-group
  • python313Packages.click-option-group.dist
  • python313Packages.cloudsplaining
  • python313Packages.cloudsplaining.dist
  • python313Packages.semgrep
  • python313Packages.semgrep.dist
  • python313Packages.subliminal
  • python313Packages.subliminal.dist
  • python313Packages.typed-settings
  • python313Packages.typed-settings.dist
  • python313Packages.whispers
  • python313Packages.whispers.dist
  • route-detect
  • route-detect.dist
  • swaggerhole
  • swaggerhole.dist
  • synadm
  • synadm.dist

aarch64-darwin

⏩ 4 packages marked as broken and skipped:
  • python312Packages.bentoml
  • python312Packages.bentoml.dist
  • python313Packages.bentoml
  • python313Packages.bentoml.dist
❌ 4 packages failed to build:
  • python312Packages.spsdk
  • python312Packages.spsdk.dist
  • python313Packages.spsdk
  • python313Packages.spsdk.dist
✅ 35 packages built:
  • checkov
  • checkov.dist
  • makejinja
  • makejinja.dist
  • mpvScripts.autosub
  • python312Packages.click-option-group
  • python312Packages.click-option-group.dist
  • python312Packages.cloudsplaining
  • python312Packages.cloudsplaining.dist
  • semgrep (python312Packages.semgrep)
  • semgrep.dist (python312Packages.semgrep.dist)
  • python312Packages.subliminal
  • python312Packages.subliminal.dist
  • python312Packages.typed-settings
  • python312Packages.typed-settings.dist
  • whispers (python312Packages.whispers)
  • whispers.dist (python312Packages.whispers.dist)
  • python313Packages.click-option-group
  • python313Packages.click-option-group.dist
  • python313Packages.cloudsplaining
  • python313Packages.cloudsplaining.dist
  • python313Packages.semgrep
  • python313Packages.semgrep.dist
  • python313Packages.subliminal
  • python313Packages.subliminal.dist
  • python313Packages.typed-settings
  • python313Packages.typed-settings.dist
  • python313Packages.whispers
  • python313Packages.whispers.dist
  • route-detect
  • route-detect.dist
  • swaggerhole
  • swaggerhole.dist
  • synadm
  • synadm.dist

@mweinelt
Copy link
Member

mweinelt commented Mar 18, 2025 •
edited
Loading

Already in staging-next.

50a77df
5562267

@mweinelt mweinelt closed this Mar 18, 2025
@GaetanLepage
Copy link
Contributor Author

GaetanLepage commented Mar 18, 2025

I'm not a huge fan of having fixes and cleanup together in the same commit, but the diff looks good overall. Thanks

You are right, I have split the changes in two separate commits.

GaetanLepage added a commit to nix-community/nixvim that referenced this pull request Mar 18, 2025
@GaetanLepage
tests/none-ls: disable broken semgrep
ffa53a4 
NixOS/nixpkgs#391003 (comment)
GaetanLepage added a commit to nix-community/nixvim that referenced this pull request Mar 18, 2025
@GaetanLepage
tests/none-ls: disable broken semgrep
3b49305 
NixOS/nixpkgs#391003 (comment)
GaetanLepage added a commit to nix-community/nixvim that referenced this pull request Mar 18, 2025
@GaetanLepage
tests/none-ls: disable broken semgrep
978b1fe 
NixOS/nixpkgs#391003 (comment)
nix-infra-bot pushed a commit to nix-community/nixvim that referenced this pull request Mar 18, 2025
@GaetanLepage @nix-infra-bot
tests/none-ls: disable broken semgrep
123d014 
NixOS/nixpkgs#391003 (comment)
GaetanLepage added a commit to nix-community/nixvim that referenced this pull request Mar 18, 2025
@GaetanLepage
tests/none-ls: disable broken semgrep
122375e 
NixOS/nixpkgs#391003 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MattSturgeon MattSturgeon MattSturgeon approved these changes

@mweinelt mweinelt Awaiting requested review from mweinelt

Assignees

No one assigned

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 11-100 This PR causes between 11 and 100 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@GaetanLepage @mweinelt @MattSturgeon