Add alias clarification to deprecation warning

[mmarinez]

@Zac-HD Yes, this is related to the issue #7815. I could come up with a better format for the warning if it is needed.

[nicoddemus]

Hi @mmarinez,

Can you clarify why the clarification is needed?

[The-Compiler]

Can you elaborate on why this is needed? I don't see how it'd be relevant for users how the private implementation of the underlying function is called. Also FWIW the tests are failing, but I don't see why this would be needed at all.

[Zac-HD]

Related to #7815?

[The-Compiler]

@mmarinez Thanks for the clarification! In the future, please clarify the context of a PR as part of the PR message when you open it - you can even use something like "Closes #7815" so that the issue is closed automatically when the PR is merged. This makes it easier for people to keep track of things, as there are a lot of stuff going on in parallel in a project like this.

[The-Compiler]

I don't really understand #7815 (though I only took a quick look), so I'll let someone else take a closer look at this. However, note that test_fillfuncargs_is_deprecated in testing/deprecated_test.py will need adjustments to fix the tests.

[mmarinez]

I don't really understand #7815 (though I only took a quick look), so I'll let someone else take a closer look at this. However, note that test_fillfuncargs_is_deprecated in testing/deprecated_test.py will need adjustments to fix the tests.

Got it, thanks for letting me know, I'll be checking that out.

[bluetech]

LGTM once CI passes, thanks @mmarinez.

[nicoddemus]

Hi @mmarinez,

Thanks for working on this. I took the liberty of updating the PR a bit:

• Instead of an alias, I created an actual _pytest.fixtures._fillfuncargs() function which issues the warning.
• _pytest.fixtures.fillfixtures() no longer issues a warning, it is already an internal function.
• Because of the above, the message now points users to use _pytest.fixtures.fillfixtures() instead, which is just internal API and just passive of changing in the future as function._request._fillfixtures(), but is a bit nicer.

With this change:

import pytest

def test1():
    pytest._fillfuncargs(mock.Mock())

Produces this warning:

test_foo.py::test1
  d:\projects\pytest\.tmp\test_foo.py:7: PytestDeprecationWarning: pytest._fillfuncargs() is deprecated, use _pytest.fixtures.fillfixtures() instead if you cannot avoid reaching into internals.
    pytest._fillfuncargs(mock.Mock())

Which I think is better. 👍

@bluetech can you please take another look?

@mmarinez for the record, the test was failling previously because match= expects a regular expression, and your message contained () so it needed to be escaped with re.escape.

[bluetech]

@nicoddemus The idea is to eventually remove the _pytest.fixtures.fillfixtures function entirely as well, not just the public alias. And the issue was triggered by a plugin which actually uses the internal name. Long story short, I'd like _pytest.fixtures.fillfixtures to continue to issue a deprecation warning.

[nicoddemus]

@nicoddemus The idea is to eventually remove the _pytest.fixtures.fillfixtures function entirely as well, not just the public alias. And the issue was triggered by a plugin which actually uses the internal name. Long story short, I'd like _pytest.fixtures.fillfixtures to continue to issue a deprecation warning.

I understand, but let me make a few points why I think the current PR is better, but if you still prefer to keep _pytest.fixtures.fillfixtures() deprecated I will change it back:

• _pytest.fixtures.fillfixtures() is already private.
• We don't have a good (public) alternative to point users to to show in the deprecation message.
• function._request._fillfixtures() is even easier to break by accident.

If we had a better alternative to point to (specially a new, public usage), I would agree 100% with you, but pointing to function._request._fillfixtures() vs _pytest.fixtures.fillfixtures() are both "private and use at your own risk", with the former being slightly better.

WDYT?

[mmarinez]

Hi @mmarinez,

Thanks for working on this. I took the liberty of updating the PR a bit:

* Instead of an alias, I created an actual `_pytest.fixtures._fillfuncargs()` function which issues the warning.

* `_pytest.fixtures.fillfixtures()` no longer issues a warning, it is already an internal function.

* Because of the above, the message now points users to use `_pytest.fixtures.fillfixtures()` instead, which is just internal API and just passive of changing in the future as `function._request._fillfixtures()`, but is a bit nicer.

With this change:

import pytest

def test1():
    pytest._fillfuncargs(mock.Mock())

Produces this warning:

test_foo.py::test1
  d:\projects\pytest\.tmp\test_foo.py:7: PytestDeprecationWarning: pytest._fillfuncargs() is deprecated, use _pytest.fixtures.fillfixtures() instead if you cannot avoid reaching into internals.
    pytest._fillfuncargs(mock.Mock())

Which I think is better. 👍

@bluetech can you please take another look?

@mmarinez for the record, the test was failling previously because match= expects a regular expression, and your message contained () so it needed to be escaped with re.escape.

Hey, it's ok thank. I was actually struggling a bit trying to run the tests locally and not push every change trying to figure why were the tests failing

[bluetech]

@nicoddemus

_pytest.fixtures.fillfixtures() is already private.

Right, although there are plugins which use it.

We don't have a good (public) alternative to point users to to show in the deprecation message.

Right.

function._request._fillfixtures() is even easier to break by accident.

But that's fine? I mean this is clearly entirely private, so if someone is using that they can't expect it to be stable.

A better alternative is to use request.fixturenames, function.fixturenames, request.getfixturevalue() directly (basically what function._request._fillfixtures() does internally), these are at least more public/stable, but it's not a direct replacement.

If we had a better alternative to point to (specially a new, public usage), I would agree 100% with you, but pointing to function._request._fillfixtures() vs _pytest.fixtures.fillfixtures() are both "private and use at your own risk", with the former being slightly better.

I agree function._request._fillfixtures() is slightly better (because I want to remove _pytest.fixtures.fillfixtures() eventually, and it's more obviously private), that's why I prefer to keep pointing to it (and just remove mentioning it) and keep _pytest.fixtures.fillfixtures() deprecated.

[nicoddemus]

I agree function._request._fillfixtures() is slightly bette

Hehehe I meant the latter.

No worries, I will re-add the deprecation warning and improve the message. 👍

[nicoddemus]

Done! 👍

[bluetech]

Thanks @mmarinez @nicoddemus.

[nicoddemus]

Backport: #7867