tests: check that PYTEST_ADDOPTS is not processed twice

[blueyed]

Since 953a3f0 options are parsed already, e.g. "-v" from
$PYTEST_ADDOPTS, and therefore $PYTEST_ADDOPTS should be unset for
child processes.

[blueyed]

What about this note?
https://github.com/pytest-dev/pytest/blob/8db965c401dd46ff9438dd114b538197d0c31c43/src/_pytest/config/__init__.py#L668-L671

I.e. with pytest-xdist it becomes part of the invocation now.
And plugins handling PYTEST_ADDOPTS in a special way won't see it anymore with this PR.

[blueyed]

Instead of unsetting it in pytest-xdist, pytest could also skip the env if invocation params are used (https://github.com/pytest-dev/pytest/blob/8db965c401dd46ff9438dd114b538197d0c31c43/src/_pytest/config/__init__.py#L870-L877). While that seems a bit magical, it might be a good/better solution.

[nicoddemus]

Thanks!

[blueyed]

@nicoddemus
Any input on my remarks?

[nicoddemus]

I.e. with pytest-xdist it becomes part of the invocation now.
And plugins handling PYTEST_ADDOPTS in a special way won't see it anymore with this PR.

That's fine, the note is about the fact that InvocationParams already contains the options from PYTEST_ADDOPTS. I think your approach is sound. @RonnyPfannschmidt?

Instead of unsetting it in pytest-xdist, pytest could also skip the env if invocation params are used

Not sure what you mean, can you please clarify?

[nicoddemus]

That's fine, the note is about the fact that InvocationParams already contains the options from PYTEST_ADDOPTS.

Actually I think I'm completely wrong here, InvocationParams is supposed to contain the unaltered command-line options passed to pytest.main()... I'll have to think about this later, now it's way past a reasonable bed time. :/

[RonnyPfannschmidt]

I like this, but I think we should investigate parameters layouts
there is env and config based addopts,

We need a alternative to pytest. Main that has the control of all of them at the call site

[nicoddemus]

OK I figure it out, the problem is that InvocationParams.args is a reference to the original args, and the PYTEST_ADDOPTS processing changes it in-place:

https://github.com/pytest-dev/pytest/blob/2e11ea61088a3d24da96205627c8bc4dcf3a70ff/src/_pytest/config/__init__.py#L867-L874

    def _preparse(self, args, addopts=True):
        if addopts:
            env_addopts = os.environ.get("PYTEST_ADDOPTS", "")
            if len(env_addopts):
                args[:] = (
                    self._validate_args(shlex.split(env_addopts), "via PYTEST_ADDOPTS")
                    + args
                )

That's why PYTEST_ADDOPTS ends up being "processed" twice.

The proper fix is to change InvocationParams.args to always be a copy, or even better, a tuple, to ensure immutability: pytest-dev/pytest#6008. With that patch, your test passes. 👍

[nicoddemus]

After pytest-dev/pytest#6008 is merged, I think we should still get your test in, as it tightens it.

[RonnyPfannschmidt]

lovely find and fix

[blueyed]

Amended to keep the test only - expected to pass on pytestfeatures only for now.

[nicoddemus]

expected to pass on pytestfeatures only for now.

Can we perhaps skip the test if InvocationParms.args is not a tuple? 🤔

[blueyed]

Yeah, but no hurry I guess.