Skip to content

(FM-8199) - Port module to Litmus #345

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 2, 2020
Merged

(FM-8199) - Port module to Litmus #345

merged 2 commits into from
Mar 2, 2020

Conversation

david22swan
Copy link
Member

  • Litmus module temporarily pinned on branch while it waits for merge

@david22swan david22swan requested a review from a team as a code owner January 31, 2020 12:49
@codecov-io
Copy link

codecov-io commented Jan 31, 2020 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@0b7b5ed). Click here to learn what that means.
The diff coverage is 35.71%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #345   +/-   ##
=========================================
  Coverage          ?   56.25%           
=========================================
  Files             ?       20           
  Lines             ?      759           
  Branches          ?        0           
=========================================
  Hits              ?      427           
  Misses            ?      332           
  Partials          ?        0
Impacted Files Coverage Δ
lib/puppet_x/sqlserver/sql_connection.rb 35.71% <35.71%> (ø)

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 0b7b5ed...67d7117. Read the comment docs.

@david22swan david22swan changed the title {WIP} (FM-8199) - Port module to Litmus (FM-8199) - Port module to Litmus Jan 31, 2020
sanfrancrisko
sanfrancrisko reviewed Feb 3, 2020
View reviewed changes
Copy link
Contributor

@sanfrancrisko sanfrancrisko left a comment

Choose a reason for hiding this comment

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

Nice job @david22swan 👍

@david22swan david22swan changed the title (FM-8199) - Port module to Litmus (WIP)(FM-8199) - Port module to Litmus Feb 3, 2020
@david22swan david22swan force-pushed the FM-8199 branch 12 times, most recently from 94b83af to 8a66c96 Compare February 7, 2020 11:42
@david22swan david22swan force-pushed the FM-8199 branch 6 times, most recently from 5dde0cc to d84afaa Compare February 13, 2020 13:48
sanfrancrisko
sanfrancrisko previously approved these changes Feb 14, 2020
View reviewed changes
Copy link
Contributor

@sanfrancrisko sanfrancrisko left a comment

Choose a reason for hiding this comment

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

LGTM overall. Probably worth another set of eyes on it though, given the size of the PR and my inexperience with Litmus ports

.sync.yml
@@ -31,20 +31,6 @@ appveyor.yml:
CHECK: parallel_spec
simplecov: true
Gemfile:
required:
":system_tests":
- gem: puppet-module-posix-system-r#{minor_version}
Copy link
Contributor

Choose a reason for hiding this comment

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

More of a query because I was working with this gem recently - should they not be in all modules? I'm guessing the PDK removed this?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's still included in :development. :system_tests is just no longer a thing with litmus

michaeltlombardi
michaeltlombardi suggested changes Feb 14, 2020
View reviewed changes
Copy link
Contributor

@michaeltlombardi michaeltlombardi left a comment

Choose a reason for hiding this comment

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

One fairly rippling change, some inline questions, and one suite question: does this work when testing against vagrant/localhost, or only for vmpooler?

@david22swan david22swan changed the title (WIP)(FM-8199) - Port module to Litmus (FM-8199) - Port module to Litmus Feb 14, 2020
@david22swan david22swan force-pushed the FM-8199 branch 2 times, most recently from efa08d7 to 7b9a886 Compare February 17, 2020 09:17
@david22swan david22swan force-pushed the FM-8199 branch 2 times, most recently from 6012e40 to 3f85a6f Compare February 18, 2020 15:38
(FM-8199) - Github actions set to run on release branch
67d7117 
- Test’s currently set to run in parity with Jenkins, further work will be done to enable full functionality testing
michaeltlombardi
michaeltlombardi approved these changes Mar 2, 2020
View reviewed changes
Copy link
Contributor

@michaeltlombardi michaeltlombardi left a comment

Choose a reason for hiding this comment

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

This litmusification does not seem to support folks outside of Puppet running the tests (depends on access to the VPN/internal resources) but, as this was true before and now people can at least see the test results publicly, this is definitely a great improvement!

@michaeltlombardi michaeltlombardi merged commit 8d44d62 into puppetlabs:master Mar 2, 2020
@david22swan david22swan deleted the FM-8199 branch December 8, 2020 14:06
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 35.71429% with 36 lines in your changes missing coverage. Please review.

Project coverage is 56.25%. Comparing base (779aa56) to head (67d7117).

Files with missing lines Patch % Lines
lib/puppet_x/sqlserver/sql_connection.rb 35.71% 36 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #345      +/-   ##
==========================================
+ Coverage   56.20%   56.25%   +0.05%     
==========================================
  Files          20       20              
  Lines         758      759       +1     
==========================================
+ Hits          426      427       +1     
  Misses        332      332

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@michaeltlombardi michaeltlombardi michaeltlombardi approved these changes

@sanfrancrisko sanfrancrisko sanfrancrisko left review comments

Assignees
No one assigned
Labels
maintenance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@david22swan @codecov-io @codecov-commenter @michaeltlombardi @sanfrancrisko