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

Some GitAPITestCase tests were replaced in GitClientTest #504

Merged
merged 9 commits into from
Feb 14, 2020

Conversation

loghijiaha
Copy link
Contributor

@loghijiaha loghijiaha commented Feb 10, 2020

JENKINS-60940 - Tests were added to GItClientTest

Checklist

  • I have read the CONTRIBUTING doc
  • I have referenced the Jira issue related to my changes in one or more commit messages
  • I have added tests that verify my changes
  • Unit tests pass locally with my changes
  • I have added documentation as necessary
  • No Javadoc warnings were introduced with my changes
  • No spotbugs warnings were introduced with my changes
  • I have interactively tested my changes

Types of changes

What types of changes does your code introduce? Put an x in the boxes that apply

  • Infrastructure change (non-breaking change which updates dependencies or improves infrastructure)

Further comments

Junit 3 tests can be sorted according to alphabet order in GitAPITestCase and compared with GItClientTestCases. It would be an efficient idea to change the tests to Junit 4.

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 very much! I believe the commit is missing from the repository that is being created in the replaced test.

I've also asked a question about the exception in the test that is being transformed. The previous test only asserted the existence of an exception. There are many different issues that may be placed into the single GitException. I'd appreciate very much if you'd be willing to assert more specifically about the contents of the GitException.

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 for reworking the clone and for the additional tests. A few more changes needed

@MarkEWaite MarkEWaite added the test Automated test addition or improvement label Feb 14, 2020
@MarkEWaite MarkEWaite self-assigned this Feb 14, 2020
@MarkEWaite
Copy link
Contributor

This looks great! Thanks very much @loghijiaha

@MarkEWaite MarkEWaite merged commit b13c40f into jenkinsci:master Feb 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Automated test addition or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants