Skip to content

Remove custom pytest hook and move pytest markers from pulp-smash#4177

Merged
dkliban merged 1 commit into
pulp:mainfrom
ATIX-AG:remove-custom-pytest-hook
Jul 31, 2023
Merged

Remove custom pytest hook and move pytest markers from pulp-smash#4177
dkliban merged 1 commit into
pulp:mainfrom
ATIX-AG:remove-custom-pytest-hook

Conversation

@hstct

@hstct hstct commented Jul 28, 2023

Copy link
Copy Markdown
Contributor

[noissue]

@mdellweg mdellweg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although we could try to make this hook implementation optional, i think this is what we discussed, and we can always bring it back once we sorted out the pulp-smash story.

@hstct

hstct commented Jul 28, 2023

Copy link
Copy Markdown
Contributor Author

This hook will cause the CI pipeline to fail and don't run any functional tests if you don't install pulp-smash as a test requirement. From the matrix chat it seems that it is not really used anyway and the consensus was to just remove it.

It will run into this error here

With the removal the tests now run again: pulp/pulp_deb#851

@dkliban

dkliban commented Jul 28, 2023

Copy link
Copy Markdown
Member

is this not used anywhere?

@mdellweg

Copy link
Copy Markdown
Member

is this not used anywhere?

@dkliban not in any CI at least (It contradicts the parallel execution of tests).
If you are a regular user of this feature, we need to find a way to make the implementation of this hook dependent of the existence of its declaration (currently in pulp-smash).

@lubosmj

lubosmj commented Jul 31, 2023

Copy link
Copy Markdown
Contributor

Can we also move this from pulp_smash: https://github.com/pulp/pulp-smash/blob/2ded6671e0f32c51e70681467199e8a195f7f5fe/pulp_smash/pulp3/pytest_plugin/__init__.py#L76? I think @hstct mentioned that without that, he sees some warnings in the pulp_deb's CI.

@mdellweg

Copy link
Copy Markdown
Member

Can we also move this from pulp_smash: https://github.com/pulp/pulp-smash/blob/2ded6671e0f32c51e70681467199e8a195f7f5fe/pulp_smash/pulp3/pytest_plugin/__init__.py#L76? I think @hstct mentioned that without that, he sees some warnings in the pulp_deb's CI.

Only the second half of it, but yes, we should.

@hstct

hstct commented Jul 31, 2023

Copy link
Copy Markdown
Contributor Author

Can we also move this from pulp_smash: https://github.com/pulp/pulp-smash/blob/2ded6671e0f32c51e70681467199e8a195f7f5fe/pulp_smash/pulp3/pytest_plugin/__init__.py#L76? I think @hstct mentioned that without that, he sees some warnings in the pulp_deb's CI.

Only the second half of it, but yes, we should.

I can include this to this PR or open a new one whatever is preferred 🙂

@lubosmj

lubosmj commented Jul 31, 2023

Copy link
Copy Markdown
Contributor

Include it in this one, please.

@hstct hstct force-pushed the remove-custom-pytest-hook branch from 7afe5a6 to 6737c9a Compare July 31, 2023 12:54
@hstct hstct changed the title Remove custom pytest hook checking for leftover pulp objects Remove custom pytest hook and move pytest markers from pulp-smash Jul 31, 2023
@dkliban dkliban merged commit ed33140 into pulp:main Jul 31, 2023
@hstct hstct deleted the remove-custom-pytest-hook branch July 31, 2023 14:17
@patchback

patchback Bot commented Aug 1, 2023

Copy link
Copy Markdown

Backport to 3.29: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.29/ed3314079e7e199ae99db4e1f05ed1668e4664f7/pr-4177

Backported as #4239

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@patchback

patchback Bot commented Aug 2, 2023

Copy link
Copy Markdown

Backport to 3.28: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.28/ed3314079e7e199ae99db4e1f05ed1668e4664f7/pr-4177

Backported as #4246

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants