Adds a pytest local fixture for pulp_file #1728

bmbouter · 2021-11-22T21:32:36Z

closes #9623

pulpbot · 2021-11-22T21:41:07Z

WARNING!!! This PR is not attached to an issue. In most cases this is not advisable. Please see our PR docs for more information about how to attach this PR to an issue.

pulpcore/tests/conftest.py

pulpcore/tests/functional/api/using_plugin/test_sync.py

bmbouter · 2021-12-07T23:10:43Z

To run the new test you can run: pytest -v -r sx --color=yes --pyargs pulpcore.tests.functional.api.using_plugin.test_sync

bmbouter · 2021-12-08T21:48:19Z

pulpcore/tests/functional/api/using_plugin/test_sync.py

+    basic_file_fixture_gen_remote,
+    file_repo,
+    file_repo_api_client,
+    content_file_api_client,
+    async_monitor_task,
+    delete_orphans_pre_post,


I started to make these pytest autouse fixtures, but it would really take away from the explicitness of this. So I left it as-is for now.

bmbouter · 2021-12-08T21:55:13Z

pulpcore/tests/conftest_pulp_file.py

+    return Path(__file__).parent / "fixtures"
+
+
+@pytest.fixture


It wouldn't let me make this one scope="session" because pytest wouldn't run with ScopeMismatch: You tried to access the 'function' scoped fixture 'aiohttp_server' with a 'session' scoped request object, involved factories.

Through more discussion we decided that while it is tempting to make each fixture server a session scope, threads are light, and eventually we want to run these in parallel which would put us back into a fixture scoped webserver. So we'll leave as fixture scope now too.

What's the difference between the scopes? What difference(benefit) would it make to have this function be a different scope?

Session level for example causes the fixture to be run once for the whole session (all the tests) so that data is cached and read from heavily. Other fixtures though we want to be per-test, e.g. the webservers themselves (and all the fixtures that support those). https://docs.pytest.org/en/latest/how-to/fixtures.html#scope-sharing-fixtures-across-classes-modules-packages-or-session

.ci/ansible/start_container.yaml

pulpcore/tests/conftest_pulp_file.py

mdellweg · 2021-12-14T10:00:15Z

pulpcore/tests/conftest.py

@@ -0,0 +1 @@
+from .conftest_pulp_file import *  # noqa


I think we should require the pulp-smash parts here:
https://pytest.org/en/latest/how-to/plugins.html#requiring-loading-plugins-in-a-test-module-or-conftest-file

Maybe, or we could just move that code here and avoid the import. The idea here was to organize the code we deliver into various submodules that then import here. It looks strange with just one of them though. Should I move the code from conftest_pulp_file here or leave as is?

I guess the only other imports here would be pulpcore related, maybe they could be useful.

I'm not talking about imports. I mean
pytest_plugins = ("pulp_smash.pulp3.pytest_plugin",)
and i keep wondering how it can even work without it.

Yeah it's confusing, but this installation uses the entrypoint method. That is in the setup.py now.

pulpcore/tests/conftest_pulp_file.py

bmbouter · 2021-12-15T18:10:54Z

Alright I think these achieve everything I'm happy with so I'm going to undraft it and leave a few comments for reviewers.

.ci/ansible/smash-config.json

bmbouter · 2021-12-16T15:41:49Z

pulpcore/tests/functional/api/using_plugin/test_sync.py

+    )
+
+
+def test_ondemand_to_immediate_sync(


This was the original test, just now rewritten.

Can you move the original class docs string here? Also, I know the function names are pretty descriptive, but do you think they deserve doc strings as well? I'm surprised the linter didn't throw a fit over the absence.

I can add some docstrings. Also I'd like to update the content for the test that was rewritten. Let me do some of that.

I added some. I try to keep them short but clear.

bmbouter · 2021-12-16T15:56:20Z

pulpcore/tests/conftest_pulp_file.py

+
+
+@pytest.fixture
+def file_fixture_gen_remote_ssl(


This is the fixture I expect most tests to use.

gerrod3 · 2021-12-16T15:57:49Z

pulpcore/tests/conftest_pulp_file.py

+    return Path(__file__).parent / "fixtures"
+
+
+@pytest.fixture


What's the difference between the scopes? What difference(benefit) would it make to have this function be a different scope?

gerrod3 · 2021-12-16T16:00:20Z

pulpcore/tests/conftest.py

@@ -0,0 +1 @@
+from .conftest_pulp_file import *  # noqa


I guess the only other imports here would be pulpcore related, maybe they could be useful.

pulpcore/tests/functional/api/using_plugin/test_sync.py

gerrod3 · 2021-12-16T16:10:20Z

pulpcore/tests/functional/api/using_plugin/test_sync.py

+    )
+
+
+def test_http_sync_ssl_tls_validation_defaults_to_on(


How does this validate that tls validation is defaulted to on?

Because the remote it constructs does not specify it (that's the default part) yet it's connecting to a TLS context that is presenting a cert that will not be trusted without additional CA configuration.

gerrod3 · 2021-12-16T16:13:45Z

pulpcore/tests/functional/api/using_plugin/test_sync.py

+    assert len(requests_record) == 1
+    assert requests_record[0].path == "/basic/PULP_MANIFEST"


Yeah this interface is so much better. Big thanks to @mdellweg for suggesting we do it this way.

Required PR: pulp/pulp-smash#1262 closes #9623

MANIFEST.in

mdellweg · 2021-12-17T14:46:41Z

pulpcore/tests/conftest.py

@@ -0,0 +1 @@
+from .conftest_pulp_file import *  # noqa


I'm not talking about imports. I mean
pytest_plugins = ("pulp_smash.pulp3.pytest_plugin",)
and i keep wondering how it can even work without it.

pulpcore/tests/conftest_pulp_file.py

gerrod3 reviewed Nov 22, 2021

View reviewed changes

pulpcore/tests/conftest.py Outdated Show resolved Hide resolved

gerrod3 reviewed Nov 22, 2021

View reviewed changes

pulpcore/tests/conftest.py Outdated Show resolved Hide resolved

mdellweg reviewed Nov 24, 2021

View reviewed changes

pulpcore/tests/functional/api/using_plugin/test_sync.py Outdated Show resolved Hide resolved

mdellweg reviewed Nov 24, 2021

View reviewed changes

pulpcore/tests/functional/api/using_plugin/test_sync.py Outdated Show resolved Hide resolved

bmbouter force-pushed the add-aiohttp-fixture branch from 53adb22 to 68313f3 Compare December 7, 2021 23:09

bmbouter force-pushed the add-aiohttp-fixture branch 6 times, most recently from 0ad113c to 12b6ea4 Compare December 8, 2021 21:34

bmbouter commented Dec 8, 2021

View reviewed changes

bmbouter force-pushed the add-aiohttp-fixture branch from 12b6ea4 to 71a9a59 Compare December 8, 2021 21:54

bmbouter commented Dec 8, 2021

View reviewed changes

bmbouter force-pushed the add-aiohttp-fixture branch 3 times, most recently from f6330c3 to e0e642a Compare December 9, 2021 21:27

dkliban reviewed Dec 9, 2021

View reviewed changes

.ci/ansible/start_container.yaml Outdated Show resolved Hide resolved

bmbouter force-pushed the add-aiohttp-fixture branch 2 times, most recently from c6bdf81 to 5f4b3d4 Compare December 9, 2021 21:51

bmbouter commented Dec 9, 2021

View reviewed changes

pulpcore/tests/conftest_pulp_file.py Outdated Show resolved Hide resolved

bmbouter commented Dec 9, 2021

View reviewed changes

pulpcore/tests/conftest_pulp_file.py Outdated Show resolved Hide resolved

bmbouter force-pushed the add-aiohttp-fixture branch 4 times, most recently from 806e460 to 316e1f6 Compare December 13, 2021 21:29

mdellweg reviewed Dec 14, 2021

View reviewed changes

pulpcore/tests/conftest_pulp_file.py Outdated Show resolved Hide resolved

bmbouter commented Dec 15, 2021

View reviewed changes

pulpcore/tests/conftest_pulp_file.py Outdated Show resolved Hide resolved

bmbouter force-pushed the add-aiohttp-fixture branch from 5837577 to 5383e74 Compare December 15, 2021 17:49

bmbouter marked this pull request as ready for review December 15, 2021 18:10

bmbouter force-pushed the add-aiohttp-fixture branch 10 times, most recently from 4121832 to e460884 Compare December 15, 2021 21:15

bmbouter commented Dec 15, 2021

View reviewed changes

.ci/ansible/smash-config.json Outdated Show resolved Hide resolved

bmbouter force-pushed the add-aiohttp-fixture branch from e460884 to 5a301da Compare December 16, 2021 15:41

bmbouter commented Dec 16, 2021

View reviewed changes

gerrod3 reviewed Dec 16, 2021

View reviewed changes

bmbouter force-pushed the add-aiohttp-fixture branch 3 times, most recently from 49116f2 to 33e8a72 Compare December 16, 2021 17:08

Adds a pytest local fixture for pulp_file

5dfb7d1

Required PR: pulp/pulp-smash#1262 closes #9623

bmbouter force-pushed the add-aiohttp-fixture branch from 33e8a72 to 5dfb7d1 Compare December 16, 2021 19:26

mdellweg reviewed Dec 17, 2021

View reviewed changes

mdellweg approved these changes Dec 17, 2021

View reviewed changes

dkliban approved these changes Dec 17, 2021

View reviewed changes

bmbouter merged commit 70b7f31 into pulp:main Dec 17, 2021

bmbouter deleted the add-aiohttp-fixture branch December 17, 2021 16:57

fao89 mentioned this pull request Jan 17, 2022

As a developer, I have a pytest fixture that allows me to run a webserver for serving that fixture to Pulp #2086

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a pytest local fixture for pulp_file #1728

Adds a pytest local fixture for pulp_file #1728

bmbouter commented Nov 22, 2021 •

edited

Loading

pulpbot commented Nov 22, 2021

bmbouter commented Dec 7, 2021 •

edited

Loading

bmbouter Dec 8, 2021

bmbouter Dec 8, 2021

bmbouter Dec 15, 2021

gerrod3 Dec 16, 2021

bmbouter Dec 16, 2021

mdellweg Dec 14, 2021

bmbouter Dec 14, 2021

gerrod3 Dec 16, 2021

mdellweg Dec 17, 2021

bmbouter Dec 17, 2021

bmbouter commented Dec 15, 2021

bmbouter Dec 16, 2021

gerrod3 Dec 16, 2021

bmbouter Dec 16, 2021

bmbouter Dec 16, 2021

bmbouter Dec 16, 2021

gerrod3 Dec 16, 2021

gerrod3 Dec 16, 2021

gerrod3 Dec 16, 2021

bmbouter Dec 16, 2021

gerrod3 Dec 16, 2021

bmbouter Dec 16, 2021

mdellweg Dec 17, 2021

		assert len(requests_record) == 1
		assert requests_record[0].path == "/basic/PULP_MANIFEST"

		)


		def test_ondemand_to_immediate_sync(



		@pytest.fixture
		def file_fixture_gen_remote_ssl(

		)


		def test_http_sync_ssl_tls_validation_defaults_to_on(

Adds a pytest local fixture for pulp_file #1728

Adds a pytest local fixture for pulp_file #1728

Conversation

bmbouter commented Nov 22, 2021 • edited Loading

pulpbot commented Nov 22, 2021

bmbouter commented Dec 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmbouter commented Dec 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmbouter commented Nov 22, 2021 •

edited

Loading

bmbouter commented Dec 7, 2021 •

edited

Loading