devhub-tud / devhub

DevHub is a software system designed to give students a simple practical introduction into modern software development.
15 stars 8 forks source link

Upgrade Mockito to 2.8.3 #432

Closed TimvdLippe closed 7 years ago

TimvdLippe commented 7 years ago

This PR upgrades Mockito to 2.8.3. It migrates deprecated method calls, removes usage of the MockitoJUnitRule and removes unused stubbings as caught by strict stubbing.

TimvdLippe commented 7 years ago

Hm, it seems to be breaking on the Mockito installation. It resolves the artifact to https://devhub.ewi.tudelft.nl/nexus/content/repositories/releases/org/mockito/mockito-core/2.8.3/mockito-core-2.8.3.pom which is no maven central. Running locally works just fine. Unsure what I can do here?

jwgmeligmeyling commented 7 years ago

The https://devhub.ewi.tudelft.nl/nexus repository falls back to maven central. Based on maven-repository.com, the latest version available on central is 2.7.22. So it seems you're testing against a local snapshot after all? Or perhaps the artifact moved to another group id?

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.3%) to 56.164% when pulling 148f721c63ce3be8bc7cc3aca4dd19c1c4609e2b on TimvdLippe:mockito-upgrade into 32643c54ae4de4032aebe0ec25a5d0ec25c8dedb on devhub-tud:master.

TimvdLippe commented 7 years ago

We can just use the silent version of the runner :) Seems like an appropriate option here

On Mon, 17 Apr 2017, 23:15 Jan-Willem Gmelig Meyling, < notifications@github.com> wrote:

@JWGmeligMeyling commented on this pull request.

In src/test/java/nl/tudelft/ewi/devhub/modules/MockedGitoliteGitServerModule.java https://github.com/devhub-tud/devhub/pull/432#discussion_r111823642:

@@ -126,7 +126,6 @@ protected void createMockedMirrorsFolder() { protected void createMockedRepositoriesFolder() { String repositoriesPath = repositoriesFolder.toPath().toString() + "/"; when(configuration.getGitoliteBaseUrl()).thenReturn(repositoriesPath);

  • when(configuration.getRepositoriesDirectory()).thenReturn(repositoriesFolder);

Aha hm that's an interesting question. Because this module was never designed for mocking unit tests, it is designed to act as a git server accessed through a git client by binding some of the git server implementation to its client interfaces. I see why Mockito discourages strict stubbing for unit tests, but really Mockito was used here primarily to:

  1. Allow tests to spy behaviour on some of the mocked behaviour
  2. Not have to go through extending every single interface in the hierarchy ourself

Perhaps it's better to rewrite this module so that it doesn't use Mockito anymore? Because I can imagine future tests that use this module failing because not all stubs will be hit in every single tests (as obviously not every possible git server interaction is used in every test). What do you think?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/devhub-tud/devhub/pull/432#discussion_r111823642, or mute the thread https://github.com/notifications/unsubscribe-auth/AFrDb5nIDa_-TqTBMZQxoRfl-g-ND6shks5rw9Z_gaJpZM4M-vUx .

jwgmeligmeyling commented 7 years ago

Perhaps because this test is only run on mvn integration-test?

TimvdLippe commented 7 years ago

Yeah, already pushed an update. Should pass soonTM

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.4%) to 56.239% when pulling 8a74d91245f1bcdf5d8aaaa9f6aab3861c2dc6b9 on TimvdLippe:mockito-upgrade into 32643c54ae4de4032aebe0ec25a5d0ec25c8dedb on devhub-tud:master.