when "disabled", keyring uses a "null" keyring and should be unavaila…

[lakinwecker]

…ble.

Pull Request Check List

Resolves: #5250

• Added tests for changed code.
• Updated documentation for changed code.

I didn't see any tests or docs for this part of the code, happy to update them as required.

[abn]

@lakinwecker can you add some regression test coverage for this?

[lakinwecker]

@abn I looked for tests related to this stuff and failed to find any. Are there any other unit tests that do things similarily that I can follow to make tests? If so, I can try and add them, yes.

[radoering]

@lakinwecker Tests are in test_password_manager.py. Probably, you should add two tests similar to

poetry/tests/utils/test_password_manager.py

Lines 190 to 195 in 15f6ec1

	def test_keyring_with_chainer_backend_and_not_compatible_only_should_be_unavailable(
	with_chained_keyring: None,
	):
	key_ring = KeyRing("poetry")
	assert not key_ring.is_available()

one with a simple null and one with a chained null. (And maybe you should rename the fixture with_chained_keyring to with_chained_fail_keyring so you can add with_chained_null_keyring). The fixtures are here:

poetry/tests/conftest.py

Lines 109 to 127 in 15f6ec1

	@pytest.fixture()
	def with_fail_keyring() -> None:
	import keyring
	from keyring.backends.fail import Keyring
	keyring.set_keyring(Keyring())
	@pytest.fixture()
	def with_chained_keyring(mocker: MockerFixture) -> None:
	from keyring.backends.fail import Keyring
	mocker.patch("keyring.backend.get_all_keyring", [Keyring()])
	import keyring
	from keyring.backends.chainer import ChainerBackend
	keyring.set_keyring(ChainerBackend())

[lakinwecker]

test_keyring_with_chainer_backend_and_not_compatible_only_should_be_unavailable doesn't fail when you mess with the "fail" code that I modified. So I'm not convinced this test properly exercises it.

[lakinwecker]

Alright, the mocks were wrong for those chainer tests. I fixed them and now the tests fail if I change the name of the 'bad' keyrings in the password_manager.py checks. So, I think this adds to tests to properly exercise it.

Is there documentation I should also update?

[lakinwecker]

I fixed the formatting issues, I'm happy to squash the commits if you prefer. Otherwise let me know if there is anything else you need for this to be reviewed.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.