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

[JENKINS-70560] Improve JVMMismatchClause test coverage #283

Merged
merged 8 commits into from
Dec 11, 2024

Conversation

code-arnab
Copy link
Contributor

Improved Test Coverage of JVMVersionMonitor.JVMMismatchCause from 0% to 72%.

Added extensive test coverage for the getTrigger function in the JVMMismatchCause Class of the enclosing JVMVersionMonitor Class. This PR will improve the test coverage of the above said function from 0% to 72%, and overall the entire test coverage of this plugin is improved by 2% from 79% to 81%.

Before

Screenshot (163)
Screenshot (164)
Screenshot (162)

After

Screenshot (159)
Screenshot (160)
Screenshot (161)

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@code-arnab code-arnab requested a review from a team as a code owner December 11, 2024 17:33
@github-actions github-actions bot added the tests Automated test addition or improvement label Dec 11, 2024
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Pull request needs to run mvn spotless:apply to retain the correct code formatting

@MarkEWaite MarkEWaite changed the title [Jenkins-70560] Improved Test Coverage og JVMVersionMonitor.JVMMismatchClause [JENKINS-70560] Improve JVMVersionMonitor.JVMMismatchClause test coverage Dec 11, 2024
@MarkEWaite MarkEWaite changed the title [JENKINS-70560] Improve JVMVersionMonitor.JVMMismatchClause test coverage [JENKINS-70560] Improve JVMMismatchClause test coverage Dec 11, 2024
…t.java


Covered toString()

Co-authored-by: Mark Waite <mark.earl.waite@gmail.com>
@code-arnab
Copy link
Contributor Author

Pull request needs to run mvn spotless:apply to retain the correct code formatting

Ah, I see. Thanks for the tip.

@code-arnab
Copy link
Contributor Author

@MarkEWaite
"continuous-integration/jenkins/pr-head — This commit cannot be built"
Sorry, but did I missed something here?

@MarkEWaite
Copy link
Contributor

@MarkEWaite
"continuous-integration/jenkins/pr-head — This commit cannot be built"
Sorry, but did I missed something here?

Click the "Details" link in the right hand column and it will open the failing job on ci.jenkins.io. It is failing because the code formatting does not match the automated code formatting and this plugin uses automated code formatting.

@code-arnab code-arnab closed this Dec 11, 2024
@code-arnab code-arnab reopened this Dec 11, 2024
@MarkEWaite
Copy link
Contributor

MarkEWaite commented Dec 11, 2024

Closing and reopening the pull request won't change the problem that exists in the pull request. In order to fix the problem in the pull request, you need to run mvn spotless:apply on your local copy, then commit the changes that are created by that command and push the result. That will reduce the size of the diff and allow the job to pass.

@code-arnab
Copy link
Contributor Author

I ran mvn spotless:apply but it is still failing. I guess I should create a fresh pr after running the command again

@MarkEWaite
Copy link
Contributor

MarkEWaite commented Dec 11, 2024

I ran mvn spotless:apply but it is still failing. I guess I should create a fresh pr after running the command again

No, you've discovered an error in the configuration of the plugin source code. Thanks for discovering it. Either you or I can add a new file ".gitattributes" to the root of the repository with the contents:

*.java text eol=lf
*.jelly text eol=lf
*.md text eol=lf
*.xml text eol=lf

That will tell git to always write line feed (LF) characters when it commits a change to a Java source file, a Jelly source file, a Markdown source file, or an XML source file. More information is available from the GitHub documentation.

Would you be willing to include that fix in this pull request or should I create a separate pull request to fix it ?

@code-arnab
Copy link
Contributor Author

I ran mvn spotless:apply but it is still failing. I guess I should create a fresh pr after running the command again

No, you've discovered an error in the configuration of the plugin source code. Thanks for discovering it. Either you or I can add a new file ".gitattributes" to the root of the repository with the contents:

*.java text eol=lf
*.jelly text eol=lf
*.md text eol=lf
*.xml text eol=lf

That will tell git to always write line feed (LF) characters when it commits a change to a Java source file, a Jelly source file, a Markdown source file, or an XML source file.

Would you be willing to include that fix in this pull request or should I create a separate pull request to fix it ?

That's interesting. Sure, I'll be willing to make a commit for this.

@github-actions github-actions bot added the skip-changelog Exclude from the changelog label Dec 11, 2024
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for improving the test coverage!

@MarkEWaite MarkEWaite merged commit 8f3b4f7 into jenkinsci:master Dec 11, 2024
18 checks passed
@code-arnab
Copy link
Contributor Author

Looks great! Thanks for improving the test coverage!

Thank You very much for all the support!

@code-arnab code-arnab deleted the testGetTrigger branch December 13, 2024 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Exclude from the changelog tests Automated test addition or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants