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

fix: plugin wrapper repr not helpful #114

Merged
merged 2 commits into from
Oct 12, 2023

Conversation

henryiii
Copy link
Collaborator

I was seeing

>       assert len(params.plugins) == 1
E       AssertionError: assert 2 == 1
E        +  where 2 = len([<validate_pyproject.plugins.PluginWrapper object at 0x1107e85d0>, <validate_pyproject.plugins.PluginWrapper object at 0x10fe8e810>])
E        +    where [<validate_pyproject.plugins.PluginWrapper object at 0x1107e85d0>, <validate_pyproject.plugins.PluginWrapper object at 0x10fe8e810>] = CliParams(input_file=[<_io.TextIOWrapper name='/private/var/folders/_8/xtbws09n017fbzdx9dmgnyyr0000gn/T/pytest-of-henryschreiner/pytest-7/test_parse_setuptools_distutil1/pyproject.toml' mode='r' encoding='UTF-8'>], plugins=[<validate_pyproject.plugins.PluginWrapper object at 0x1107e85d0>, <validate_pyproject.plugins.PluginWrapper object at 0x10fe8e810>], loglevel=30, dump_json=False).plugins

In the pytest failures messages. This instead produces a much more helpful:

>       assert len(params.plugins) == 1
E       AssertionError: assert 2 == 1
E        +  where 2 = len([PluginWrapper('distutils', load_builtin_plugin), PluginWrapper('scikit-build', get_skbuild_schema)])
E        +    where [PluginWrapper('distutils', load_builtin_plugin), PluginWrapper('scikit-build', get_skbuild_schema)] = CliParams(input_file=[<_io.TextIOWrapper name='/private/var/folders/_8/xtbws09n017fbzdx9dmgnyyr0000gn/T/pytest-of-henryschreiner/pytest-9/test_parse_setuptools_distutil1/pyproject.toml' mode='r' encoding='UTF-8'>], plugins=[PluginWrapper('distutils', load_builtin_plugin), PluginWrapper('scikit-build', get_skbuild_schema)], loglevel=30, dump_json=False).plugins

Which makes the problem clear (I can't have a plugin installed into the testing environment).

@henryiii
Copy link
Collaborator Author

(PS: I copied the wrong output above, it actually shows the function repr in the current implementation, instead of only the func.__name__, but same idea and the tool name was the most useful part IMO).

@henryiii
Copy link
Collaborator Author

Ah, this is useful in CI, too!

�[1m�[31mE        +  where 2 = len([PluginWrapper('distutils', <function load_builtin_plugin at 0x7fab51c88a60>), PluginWrapper('repo-review', <function get_schema at 0x7fab5163dcf0>)])�[0m

So the CI has repo-review installed, which also provides a plugin. This test seems a little fragile?

@henryiii henryiii mentioned this pull request Oct 11, 2023
@henryiii
Copy link
Collaborator Author

FYI, if you enable "always suggest updating branches" in the repo settings, I'll get a button that will allow be to rebase without having to leave the webpage. :)

@abravalheri
Copy link
Owner

abravalheri commented Oct 11, 2023

FYI, if you enable "always suggest updating branches" in the repo settings, I'll get a button that will allow be to rebase without having to leave the webpage. :)

Done. Thank you, I was not aware of that.

@abravalheri
Copy link
Owner

abravalheri commented Oct 11, 2023

So the CI has repo-review installed, which also provides a plugin. This test seems a little fragile?

Yeah, a bit. I was waiting the "3 strike rule" to change it, so for now I just patched it to ignore everything that is not validate_pyproject... but I think I saw in one of your PRs something more interesting.

@henryiii
Copy link
Collaborator Author

My solution (originally in #115) was to simply ensure that the excluded plugin was not in the list of plugins (all(x.tool != tool for ...)).

@abravalheri abravalheri merged commit 0ec8682 into abravalheri:main Oct 12, 2023
29 checks passed
@henryiii henryiii deleted the henryiii/fix/pluginrepr branch October 12, 2023 14:29
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.

2 participants