Remove pulp-smash from test_pull_content #1921
Conversation
gerrod3
force-pushed
the
smash-10
branch
3 times, most recently
from
775f08a to
3e7b210
Compare
| monitor_task( | ||
| pulpcore_bindings.OrphansCleanupApi.cleanup({"orphan_protection_time": 0}).task | ||
| ) |
There was a problem hiding this comment.
We do have the orphan_cleanup_pre fixture for this I believe.
And it even prevents you from adding the parallel mark.
There was a problem hiding this comment.
It's called delete_orphans_pre, yeah?
There was a problem hiding this comment.
I already tried, it is not allowed because of scope mismatch.
| repo = container_repository_factory() | ||
| remote = container_remote_factory(policy="on_demand") | ||
| container_sync(repo, remote) | ||
| type(self).repo = container_bindings.RepositoriesContainerApi.read(repo.pulp_href) |
There was a problem hiding this comment.
Can you explain why type(self).repo is needed vs. just self.repo?
There was a problem hiding this comment.
I'm not fully sure, but I think it has to do with how the tests in a class are executed in pytest. Basically the tests don't seem to share the same state in the self object (they might not be the same object), so for a class fixture that you want to assign a state for you must put it on the class object, i.e. type(self). It's an ugly pattern so I'll remove it.
|
|
||
|
|
||
| class PullOnDemandContentTestCase(unittest.TestCase): | ||
| class TestPullOnDemandContent: |
There was a problem hiding this comment.
Is it not possible for TestPullContent and TestPullOnDemandContent to share some of this code instead of duplicating so much of it? It looks like much of it was copied and pasted at one point.
There was a problem hiding this comment.
This shouldn't be interpreted as a blocker to the immediate goal of dropping pulp-smash ^
gerrod3
force-pushed
the
smash-10
branch
3 times, most recently
from
af7c9df to
de07814
Compare
|
@dralley Can you reapprove, got the CI green now. |
No description provided.