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

Automated tests #30

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

gd-tmagrys
Copy link
Contributor

Implementing story:

#16

This PR overrides:

#24

Please review and merge if possible.

@ctapobep
Copy link

ctapobep commented Jul 8, 2015

@gd-tmagrys the first 2 commits here are outdated, is that right?

@gd-tmagrys
Copy link
Contributor Author

Yes, because this work was created on top of #29 .
I'll rebase it on master.

Implement automated tests for plugin API HTTP resource
Add Mockito and Power Mock dependency
Implement automated tests for ConfigurationsManager. Implement additional tests for HTTP Client and HTTP resource
Exclude artifacts forbidden by maven enforce plugin
@gd-tmagrys
Copy link
Contributor Author

@ctapobep, I rebased work regarding this PR on master. Feel free to review.

@ctapobep
Copy link

ctapobep commented Jul 9, 2015

Hm. But I see classes like JerseyClientFactory added within this commit, so it's not just tests.
I haven't looked in much details at the tests, but the overall impression is still that we mock too much..

Also, couple of test design comments:

  • To follow BDD test names should state "if blah-blah then blah blah" or "blah blah should do blah blah". In this case it's clear what behavior test expects. And it's not clear what is expected in the test with name testPostForTwoMatchedRepositories
  • Tests should be small. If you have a lot of preparations, you can factor it out into separate methods. E.g. this code:
M2Repository repository = mock(M2Repository.class);
when(repository.getRemoteUrl()).thenReturn("http://localhost:8081/nexus/content/repositories/snapshots/");
when(repository.getId()).thenReturn("replica-1");
when(repository.getArtifactStoreHelper()).thenReturn(artifactStoreHelper);
repositories.add(repository);

Can be placed in method: private M2Repository repo(String id, String url). There may be couple of overloaded methods like private M2Repository repo(String id) and private M2Repository repo() that return repos with default or random IDs/URLs.

@kevstigneev kevstigneev mentioned this pull request Jul 9, 2015
@gd-tmagrys
Copy link
Contributor Author

@ctapobep, I hope that I fix things you've mentioned in my last commit.

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