Skip to content

Automated tests#30

Open
gd-tmagrys wants to merge 3 commits intogriddynamics:masterfrom
gd-tmagrys:testing
Open

Automated tests#30
gd-tmagrys wants to merge 3 commits intogriddynamics:masterfrom
gd-tmagrys:testing

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