Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix metric/ci_mixin_spec using kubevirt vm #23293

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

agrare
Copy link
Member

@agrare agrare commented Dec 10, 2024

The metric/ci_mixin_spec was using a kubeivrt vm as an example of a class which doesn't support capture.

This is no longer accurate as we have added C&U support for kubevirt, plus we shouldn't be using provider plugin classes in core specs so use stub_supports instead.

@agrare agrare requested a review from Fryguy as a code owner December 10, 2024 15:36
@agrare
Copy link
Member Author

agrare commented Dec 10, 2024

I thought about just deleting these tests, since they essentially check that a class has included a mixin that defines supports :capture and stub_supports(vm, :capture) && expect(vm.supports?(:capture)).to be_truthy isn't much of a test.

We have a similar issue with hosts further down in this spec, and stub_supports has to work on different models so we can't just do stub_supports/stub_supports_not on two instances of the same factory.

@Fryguy
Copy link
Member

Fryguy commented Dec 10, 2024

I thought about just deleting these tests, since they essentially check that a class has included a mixin that defines supports :capture and stub_supports(vm, :capture) && expect(vm.supports?(:capture)).to be_truthy isn't much of a test.

Yeah that just sounds like testing the supports feature mixin itself.

@agrare agrare force-pushed the fix_metric_ci_mixin_spec_failure branch from 1aedb61 to c44c194 Compare December 10, 2024 16:12
@agrare
Copy link
Member Author

agrare commented Dec 10, 2024

Okay I dropped these two tests, PTAL

The metric/ci_mixin_spec was using a kubeivrt vm as an example of a
class which doesn't support capture.

This is no longer accurate as we have added C&U support for kubevirt,
plus we shouldn't be using provider plugin classes in core.

These specs are essentially testing that a class that includes a module
with `supports :capture` responds to `.supports?(:capture)` with true
which isn't actually testing anything.
@agrare agrare force-pushed the fix_metric_ci_mixin_spec_failure branch from c44c194 to 41c0efd Compare December 10, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants