Add ParameterResolver tests #352
Conversation
Added more automated tests for ParameterResolverTest class
src/test/java/org/jenkinsci/plugins/badge/ParameterResolverTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/jenkinsci/plugins/badge/ParameterResolverTest.java
Outdated
Show resolved
Hide resolved
Yes originally someone before me had started with Parametirised tests so I continued with it |
|
hey @MarkEWaite can you confirm it covers more than 80% coverage because it is showing in my IDE 100% coverage. Just wanted to confirm |
Unfortunately, you are misreading the output from your IDE. It reports 100% class coverage and 100% method coverage, but only 55% line coverage and 50% branch coverage. |
|
Does it look good or Should I add more tests? Any other tips or insights are helpful |
I'm experimenting with your pull request now. I'll let you know what I learn |
Uses a JUnit 5 WithJenkins annotation to provide a JenkinsRule argument to the test method. The JenkinsRule argument provides access to a running Jenkins from within the test. The test creates a freestyle job and runs it twice. It changes the description of each of those builds and confirms that the change is resolved by the parameter resolver. Plugin documentation describes the parameter resolver https://plugins.jenkins.io/embeddable-build-status/#plugin-content-parameter-resolver that can be applied to builds and provides replacement text in the embeddable build status result based on the data from the build.
There was a problem hiding this comment.
I removed the test cases that duplicated conditions that are already tested by the existing parameterized test. I removed the test cases that were checking cases that are not mentioned in the code (like Unicode).
I added a test case that uses WithJenkins to create a test with a freestyle job that is run twice. The assertions in that test confirm that the special cases described in the documentation are handled as expected by the parameter resolver. That test increases the ParameterResolver coverage.
|
Oh ok there is so much to learn from the changes you made, is it ok if I ask for a feedback on my approach and anything you want to point out whenever you get time? Thank you for merging the changes |
|
Oh ok there is so much to learn from the changes you made, is it ok if I ask for a feedback on my approach, whenever you get time? Thank you for merging the changes |
|
I'm not sure how you approached the goal of increasing the coverage of the ParameterResolver object, but the added tests did not significantly increase the test coverage. My initial attempts were also unable to increase test coverage. I started with your tests and tried switching them from using Mock objects to using JenkinsRule without mock objects. That didn't work. When I found that I was unable to improve test coverage from the source code, I changed my approach and reviewed the plugin documentation for a reference to "parameter resolver". Based on the documentation, I experimented with a Jenkins installation and used embeddable build status to display the build number in a build status icon. Once I had that working interactively, I investigated how to implement that in an automated test. Unfortunately, I'm not sure that path would work for you or for other first time contributors. Interactive testing was a very good step for me and first time contributors can do that. Reading other tests is probably also feasible for first time contributors, though it is difficult to know which test is a good starting point. |
Can you point me to a resource where I could learn how to interact with embeddable build status and its classes/components I have locally installed Jenkins and running on my localhost because I also tried to follow through with Interactive testing but I did not know how to. So what I did was I used chatgpt to know where will the Pattern class fail or does not support certain things(like unicode). Can you or maybe someone else guide me through on how do I interactively use embeddable build on my localhost to understand it completely and write more appropriate and quality tests that would increase the coverage.
Yes I had that in mind but did not find any useful ones I should read for this class ParameterResolver |
That is a very good question. The plugin documentation provides many small details but does not show the path to access the pages with embeddable build status examples. Each Jenkins job includes an "Embeddable build status" link on its root page. They look like this:
Each build within a Jenkins job includes an "Embeddable build status" link on its page. They look like this:
If you're interested in writing some documentation, it would be nice to provide a better introduction to the embeddable build status plugin as part of its README file. That file is currently very rich with details but sadly lacking an overview of how to use the plugin and why a user would choose to use the plugin. I also understand if you're not comfortable writing documentation for the plugin or if you're not especially interested in writing documentation for the plugin. That's fine as well. A first time user is often a great source of ideas for better documentation. It is less frequent that a new user is a creator of better documentation. As an example of one way that I use the plugin, I have a status page that tells me the results of the most recent builds of plugins that interest me. If one of them is failing a build or has failing tests, I can see it very quickly on a single page. |
|
Sure I would be happy to improve the documentation for users so that they can get an overview of this plugin, but I will be needing guidance from people like you to first understand this clearly then I think I will be able to explain it more clearly in the documentation and there will be less errors or none |
|
I have found a video which is a great resource that can be used to improve the documentation here is the link. Do let me know what do you think about this? |
Added more automated tests and increased coverage for ParameterResolver class
Testing done
Submitter checklist