Pickup addition positional args passed to _parse_parametrize_args

[kevinjfoley]

Fix for #5482.

[nicoddemus]

Thanks! Please also add a CHANGELOG entry and a quick test which shows the problem. 👍

[kevinjfoley]

Will do, will also change the arg names which is causing the tests to fail currently.

Any input on where the test should be added?

[nicoddemus]

I think testing/python/metafunc.py should do it, thanks.

[nicoddemus]

Hmmm I wonder though, since this is already broken, we should make everything except argnames and argvalues keyword only in 5.0? 🤔

@asottile @blueyed @Zac-HD what do you think?

[nicoddemus]

(@kevinjfoley perhaps hold on to your patch for a bit, wouldn't want you to waste your time)

[Zac-HD]

I'm a big fan of keyword-only arguments... but we broke this in 4.6, so I'd prefer to accept the PR (thanks @kevinjfoley!) and then have a follow-up issue to work out what and how to convert to keyword-only arguments.

[codecov]

Codecov Report

Merging #5483 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #5483      +/-   ##
==========================================
+ Coverage   96.07%   96.07%   +<.01%     
==========================================
  Files         117      117              
  Lines       25538    25542       +4     
  Branches     2473     2473              
==========================================
+ Hits        24536    24540       +4     
  Misses        698      698              
  Partials      304      304

Impacted Files	Coverage Δ
src/_pytest/mark/structures.py	92.57% <ø> (ø)	⬆️
testing/python/metafunc.py	94.92% <100%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f5b078...9571443. Read the comment docs.

[kevinjfoley]

thanks @kevinjfoley

No problem!

Pushed a squashed commit with the fix and a test demonstrating the problem, let me know if anything else is needed.

[nicoddemus]

but we broke this in 4.6, so I'd prefer to accept the PR (thanks @kevinjfoley!) and then have a follow-up issue to work out what and how to convert to keyword-only arguments.

Yeah, I agree, thanks for putting me in check. 😁

[nicoddemus]

Thanks again @kevinjfoley, appreciate it!

[kevinjfoley]

Thanks again @kevinjfoley, appreciate it!

Happy to be able to contribute!

[asottile]

Hmmm i don't think this should be merged, it was a bug that parametrize swallowed extra arguments (the test is asserting that a bug exists)

[asottile]

Oh already merged 🤔

[nicoddemus]

Hmmm i don't think this should be merged, it was a bug that parametrize swallowed extra arguments (the test is asserting that a bug exists)

Not really, the extra arguments are valid, here's the signature:

pytest/src/_pytest/python.py

Line 947 in 37fce6c

def parametrize(self, argnames, argvalues, indirect=False, ids=None, scope=None):

So it is in fact a regression, I'm afraid.

[asottile]

Yikes, always assumed those were kwargonly, and it's probably too late to do that for 5.0

[nicoddemus]

I guess we should open an issue then to discuss how to change that so it emits a warning in 5.X so we can turn it into keyword-only in 6.0 at least.