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

Drop (useless?) is_expected.to compile examples #210

Closed
wants to merge 1 commit into from
Closed

Drop (useless?) is_expected.to compile examples #210

wants to merge 1 commit into from

Conversation

jay7x
Copy link
Member

@jay7x jay7x commented Oct 28, 2023

Those is_expected.to compile looks useless from my perspective because there are other examples checking for other things in the catalog. So we can see the catalog compilation is failed anyway.

The benefit here is to reduce the unit test suite execution time.

@jay7x
Copy link
Member Author

jay7x commented Oct 28, 2023

Below are some unit test benchmarks on my m1-based macbook

Before the change:
9442 examples, 0 failures, 14 pending

  • rake spec_standalone: Finished in 11 minutes 10 seconds (files took 4.44 seconds to load)
  • rake parallel_spec_standalone: Took 187 seconds (3:07)

After the change:
8182 examples, 0 failures, 14 pending

  • rake spec_standalone: Finished in 8 minutes 56 seconds (files took 4.53 seconds to load)
  • rake parallel_spec_standalone: Took 168 seconds (2:48)

@duritong
Copy link
Collaborator

Afair compile does validate relationships, I don't think other tests do that.

@duritong
Copy link
Collaborator

At least in the past compile catched issues that were not visible in other tests.

@duritong
Copy link
Collaborator

Yeah this is essentially what we are interested in and what compile gives us: https://rspec-puppet.com/documentation/classes/#test-that-the-catalogue-compiles

It will test that the manifest can be compiled into a catalogue, and that the catalogue has no dependency cycles between resources.

The second part is what the other tests don't give you afair.

@jay7x
Copy link
Member Author

jay7x commented Oct 30, 2023

Ah! Right, I didn't thought about relationship.. let's close the PR then

@jay7x jay7x closed this Oct 31, 2023
@jay7x jay7x deleted the no_compile_test branch October 31, 2023 15:16
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.

2 participants