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

Refactor RSpec tests #624

Merged
merged 1 commit into from Aug 6, 2020
Merged

Conversation

crazymind1337
Copy link
Contributor

I have rewritten all rspec tests using shared_examples. I have mostly taken the test cases from the old rspecs.

  • now every supported operating system from metadata.json is been tested
  • introduce shared_examples

I have also made a tiny bugfix which occured while writing the tests and other changes to manifests are only sorting of variables.

@crazymind1337 crazymind1337 requested a review from a team as a code owner June 12, 2020 22:44
@crazymind1337
Copy link
Contributor Author

I still have to fix rubocop-test.

@crazymind1337 crazymind1337 marked this pull request as draft June 12, 2020 22:48
@crazymind1337 crazymind1337 force-pushed the refactor/rspec branch 2 times, most recently from 62050ef to ea4d915 Compare June 12, 2020 23:35
@crazymind1337 crazymind1337 marked this pull request as ready for review June 12, 2020 23:48
@codecov-commenter
Copy link

codecov-commenter commented Jun 12, 2020

Codecov Report

Merging #624 into master will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #624   +/-   ##
=======================================
  Coverage   27.94%   27.94%           
=======================================
  Files          19       19           
  Lines         680      680           
=======================================
  Hits          190      190           
  Misses        490      490           
Impacted Files Coverage Δ
...ib/puppet/parser/functions/docker_service_flags.rb 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6666cf...fe07788. Read the comment docs.

@crazymind1337 crazymind1337 force-pushed the refactor/rspec branch 4 times, most recently from 40e0e25 to 28e6862 Compare June 14, 2020 14:23
@crazymind1337
Copy link
Contributor Author

I did some final edits after reviewing my own changes.

@crazymind1337
Copy link
Contributor Author

I am still missing some content => ... tests since I had no good idea how to test epp or erb for whole content.

@sanfrancrisko
Copy link
Contributor

@crazymind1337 Are you still intending to address the Rubocop violations and remove them from the TODO list? If so, we might want to make this PR a draft until it's ready for review?

@crazymind1337
Copy link
Contributor Author

crazymind1337 commented Jun 24, 2020

@crazymind1337 Are you still intending to address the Rubocop violations and remove them from the TODO list? If so, we might want to make this PR a draft until it's ready for review?

Oh, I really forget about that. I have moved all but one of the rubocop_todos to the .rubocop.yaml. I was able to remove one of them completly.

@crazymind1337
Copy link
Contributor Author

I am wondering why the ubuntu1604 / LitmusAcceptance-test is failing now. Any ideas?

@sanfrancrisko
Copy link
Contributor

Thanks for this PR @crazymind1337 - quite an effort, but very much appreciated. I've been reviewing a bit today, but given the size, it'll likely take another bit of time, but we'll try to get it over the line as soon as we can.

@crazymind1337
Copy link
Contributor Author

Any progress?

@sanfrancrisko
Copy link
Contributor

@crazymind1337 Apologies for the delay - I have been reviewing and testing your changes and they look good. A few last things I want to do but some other high priority stuff came in and blew everything out of the water - I'll try to get back to this early next week and merged by the end of next week.

@david22swan
Copy link
Member

@crazymind1337
Making a quick change to point this PR towards the newly defined default main branch as part of our work to remove inappropriate terminology. Apologies if this is inconvenient in any way.

@david22swan david22swan changed the base branch from master to main August 3, 2020 15:42
@crazymind1337
Copy link
Contributor Author

crazymind1337 commented Aug 4, 2020

@crazymind1337
Making a quick change to point this PR towards the newly defined default main branch as part of our work to remove inappropriate terminology. Apologies if this is inconvenient in any way.

I have rebased onto the new default branch.

@sanfrancrisko
Copy link
Contributor

Thanks for your patience @crazymind1337 - I'm rotating on triage from later today, so I'll make your PR the first priority.

@crazymind1337
Copy link
Contributor Author

Stay calm. No stress please! :D

@sanfrancrisko sanfrancrisko changed the title rewrite rspec tests Refactor RSpec tests Aug 6, 2020
@sanfrancrisko sanfrancrisko merged commit ef89ada into puppetlabs:main Aug 6, 2020
@crazymind1337
Copy link
Contributor Author

Wohoo!!

@crazymind1337 crazymind1337 deleted the refactor/rspec branch August 13, 2020 14:15
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.

None yet

4 participants