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

Add tests for ball logic in image resolver page #141

Merged
merged 21 commits into from
Dec 11, 2022

Conversation

Okhan-coder
Copy link
Contributor

@Okhan-coder Okhan-coder commented Dec 2, 2022

This merge has 3 unit tests covering Ball Logic, First Test sees if the default 32x32 image logic is correct. The second is just a regular ball, and 3rd is an empty test case. I also Updated Mockito so it works with Final classes. Still need to cover non-ball image requests but will handle that at a later time. #114

  • 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

@MarkEWaite MarkEWaite added the test label Dec 2, 2022
@MarkEWaite
Copy link
Contributor

Thanks for the pull request @Okhan-coder . In future pull requests, please create a branch for the changes instead of submitting it from your master branch. The first item in the submitter checklist says:

  • [ ] Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!

When you submit a pull request from your own master branch, you make your own development experience more complicated and you make it a little more complicated for the plugin maintainer.

@Okhan-coder
Copy link
Contributor Author

Apologies I will pay attention to that in the future thank you!

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.

I started some experiments to create comparable tests without using mocks and it seemed to be working well. I ran out of time to complete those comparable tests, but if you're interested in my "work in progress" I could push them to this pull request (on your branch). Would that be OK or would you rather I push my prototypes to branch in my own repository that you can then use for comparison without it disrupting your branch?

In case you're interested in the experiments I was doing to try to use the objects directly rather than mocking, see https://github.com/MarkEWaite/embeddable-build-status-plugin/commits/Okhan-coder/master

pom.xml Outdated Show resolved Hide resolved
@Okhan-coder
Copy link
Contributor Author

Okhan-coder commented Dec 3, 2022

I started some experiments to create comparable tests without using mocks and it seemed to be working well. I ran out of time to complete those comparable tests, but if you're interested in my "work in progress" I could push them to this pull request (on your branch). Would that be OK or would you rather I push my prototypes to branch in my own repository that you can then use for comparison without it disrupting your branch?

In case you're interested in the experiments I was doing to try to use the objects directly rather than mocking, see https://github.com/MarkEWaite/embeddable-build-status-plugin/commits/Okhan-coder/master

Pushing them to this pull request would be great thanks!

@Okhan-coder
Copy link
Contributor Author

Hello Mark, To make these tests work without mocks I had to make an extension of ImageResolver in the test. The reason for this is Color. getimageof () used the Stapler class which always returned a null pointer exception. I was unsure how to stop this from happening without mocks. If you have any solutions that don't include extensions to this let me know I can change the code.

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.

Thanks. Sorry for the long delay on my feedback.

pom.xml Outdated Show resolved Hide resolved
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.

The tests that I added would benefit from more work, but that's only peripherally part of this pull request. The test you added looks good to me.

@MarkEWaite MarkEWaite changed the title Improve automated test coverage #114 - Add test for classes for Image Resolver Page Focusing on Ball Logic Add test for ball logic in image resolver page Dec 10, 2022
@MarkEWaite MarkEWaite changed the title Add test for ball logic in image resolver page Add tests for ball logic in image resolver page Dec 10, 2022
@MarkEWaite MarkEWaite enabled auto-merge (squash) December 10, 2022 12:16
@MarkEWaite MarkEWaite merged commit c4394de into jenkinsci:master Dec 11, 2022
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