Upgrade JUnit Mockito strictness - #990
Conversation
47f31df to
03e274c
Compare
There was a problem hiding this comment.
🟡 Not ready to approve
Two WOPI tests introduce strict subject.getSession() stubs that appear unused in the exercised code paths and can trigger Mockito UnnecessaryStubbingException.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR tightens Mockito/JUnit test strictness by removing lenient() stubbings and refactoring test setup so only stubs that are actually exercised are defined, aiming to catch accidental/unverified test fixtures.
Changes:
- Replaced
Mockito.lenient()stubs with strictwhen(...)and/or moved stubbing into the specific tests that need it. - Refactored several tests to use helper “stub…” methods and more deterministic fixtures (e.g., concrete
TextFieldinstead of mock field mutation). - Split out Vite dev-server MIME-type inference into a dedicated unit test class.
File summaries
| File | Description |
|---|---|
| src/test/java/com/researchspace/webapp/integrations/wopi/WopiControllerTestMVCIT.java | Removes lenient stubbing in WOPI MVC test setup (now strict). |
| src/test/java/com/researchspace/webapp/integrations/wopi/WopiAuthorisationInterceptorTest.java | Removes lenient stubbing in WOPI interceptor test setup (now strict). |
| src/test/java/com/researchspace/webapp/integrations/snapgene/DNAViewerControllerTest.java | Refactors SnapGene stubbing into helper methods and removes lenient stubs. |
| src/test/java/com/researchspace/webapp/integrations/protocolsio/ProtocolsIOOAuthTest.java | Moves Shiro session stubbing to only the test that needs it. |
| src/test/java/com/researchspace/webapp/integrations/protocolsio/ProtocolsIOConverterTest.java | Removes lenient/unnecessary stubbing from converter tests. |
| src/test/java/com/researchspace/webapp/integrations/protocolsio/ProtocolsIOControllerTest.java | Replaces lenient stubs with per-test stubbing helpers. |
| src/test/java/com/researchspace/webapp/filter/OriginRefererCheckingInterceptorTest.java | Makes server URL stub test-local to avoid unused stubs. |
| src/test/java/com/researchspace/webapp/filter/FullPathSiteMeshFilterTest.java | Removes lenient stub and keeps strict attribute stubbing. |
| src/test/java/com/researchspace/webapp/controller/WorkspacePermissionsDTOBuilderTest.java | Removes lenient stubs and re-stubs permissions only where needed. |
| src/test/java/com/researchspace/webapp/controller/WorkspaceControllerTest.java | Converts lenient stubs to strict stubs in setup/helpers. |
| src/test/java/com/researchspace/webapp/controller/UserRoleHandlerImplTest.java | Moves role lookup stubbing into tests to avoid unused stubs. |
| src/test/java/com/researchspace/webapp/controller/UserProfileControllerTest.java | Removes lenient message stubbing that wasn’t required. |
| src/test/java/com/researchspace/webapp/controller/SysAdminControllerTest.java | Removes lenient session stubs; keeps only required stubs. |
| src/test/java/com/researchspace/webapp/controller/StructuredDocumentControllerTest.java | Removes lenient stubs and adjusts shared expectations usage. |
| src/test/java/com/researchspace/webapp/controller/SelfServiceLabGroupControllerTest.java | Replaces @BeforeEach lenient stubs with explicit per-test stubbing helpers. |
| src/test/java/com/researchspace/webapp/controller/RSChemControllerTest.java | Removes unused folder-related stubbing/locals. |
| src/test/java/com/researchspace/webapp/controller/RepositoryConfigurationControllerTest.java | Reworks integration handler stubbing to be strict and argument-based. |
| src/test/java/com/researchspace/webapp/controller/NotebookEditorControllerTest.java | Removes lenient stub in setup. |
| src/test/java/com/researchspace/webapp/controller/NfsSysAdminControllerTest.java | Refactors message-source stubbing into helper and makes exception assertions stricter. |
| src/test/java/com/researchspace/webapp/controller/MessageAndRequestControllerTest.java | Removes lenient stubbing that wasn’t used. |
| src/test/java/com/researchspace/webapp/controller/JournalControllerTest.java | Converts lenient Shiro stubs to strict stubs. |
| src/test/java/com/researchspace/webapp/controller/ExportControllerTest.java | Moves disk-space stubbing into only the test that uses it. |
| src/test/java/com/researchspace/testutils/ProdConfigWiringTest.java | Renames BeforeClass method to skipFastRuns. |
| src/test/java/com/researchspace/testutils/BaseManagerTestCaseBase.java | Renames BeforeClass method to skipFastRuns. |
| src/test/java/com/researchspace/service/StoichiometryManagerImplTest.java | Moves DAO save stubbing to explicit helper used by relevant tests. |
| src/test/java/com/researchspace/service/inventory/impl/SampleApiManagerImplSamplesToUpdateCountTest.java | Removes lenient stubs in mock template builder. |
| src/test/java/com/researchspace/service/inventory/impl/InventoryLinkManagerImplUnitTest.java | Replaces lenient setup with explicit per-test stub helpers. |
| src/test/java/com/researchspace/service/inventory/impl/InventoryLinkManagerImplReferencingTest.java | Tightens stubbing, introduces explicit identity stubbing helper. |
| src/test/java/com/researchspace/service/inventory/impl/InventoryFileApiManagerImplAttachingTest.java | Replaces lenient fixtures with explicit helpers and stricter setup. |
| src/test/java/com/researchspace/service/inventory/impl/InventoryBulkOperationHandlerGuardTest.java | Removes lenient stub in lambda-based fixture. |
| src/test/java/com/researchspace/service/inventory/ApiExtraFieldsHelperLinkUpdateTest.java | Replaces lenient stubs with strict ones. |
| src/test/java/com/researchspace/service/impl/ZipCopyTest.java | Fixes constant name typo (ZCIP → ZIP). |
| src/test/java/com/researchspace/service/impl/UserDeletionManagerTest.java | Refactors lenient stubs into explicit helpers and fixes checked exceptions. |
| src/test/java/com/researchspace/service/impl/TemplateTransferServiceTest.java | Removes lenient stubs and makes message-source stubbing explicit per test. |
| src/test/java/com/researchspace/service/impl/SystemConfigurationInitializorTest.java | Converts lenient system property lookup stub to strict. |
| src/test/java/com/researchspace/service/impl/StoichiometryInventoryLinkManagerImplTest.java | Simplifies exception assertions without changing behavior. |
| src/test/java/com/researchspace/service/impl/RecordSharingManagerImplTest.java | Refactors permission stubbing; uses a single strict stub for parsing permission strings. |
| src/test/java/com/researchspace/service/impl/OntologyDocManagerTest.java | Moves previously-lenient stubs into the tests that need them. |
| src/test/java/com/researchspace/service/impl/MovePermissionCheckerTest.java | Tightens stubbing and fixes a comment typo. |
| src/test/java/com/researchspace/service/impl/MockFolderStructure.java | Removes lenient stubs and simplifies mock-folder-structure setup. |
| src/test/java/com/researchspace/service/impl/IntegrationsHandlerTest.java | Removes lenient stubs from setup and test cases. |
| src/test/java/com/researchspace/service/impl/ImageProcessorImplTest.java | Removes lenient save stub and aligns stubbing with the test path. |
| src/test/java/com/researchspace/service/impl/BioPortalOntologiesServiceTest.java | Removes lenient client stub in a short-query test. |
| src/test/java/com/researchspace/service/impl/ApiAvailabilityHandlerImplTest.java | Converts lenient() stubs to strict when(...). |
| src/test/java/com/researchspace/service/FolderOrganisationAndApiInboxFolderTest.java | Removes lenient stubs and makes required ones test-local. |
| src/test/java/com/researchspace/service/archive/StoichiometryImportRevisionFixupManagerTest.java | Replaces mock Field mutation with concrete TextField instance. |
| src/test/java/com/researchspace/linkedelements/FieldParserTest.java | Removes lenient converter-factory stubs from tests. |
| src/test/java/com/researchspace/extmessages/msteams/MsTeamsMessageSenderTest.java | Removes lenient doc-name stub. |
| src/test/java/com/researchspace/export/pdf/SvgToPngConverterTest.java | Renames BeforeClass method to skipFastRuns. |
| src/test/java/com/researchspace/export/pdf/MSWordProcessorTest.java | Moves stoichiometry HTML generator stub into the specific test. |
| src/test/java/com/researchspace/export/pdf/ImageRetrieverHelperTest.java | Converts lenient permission stubs to strict and refactors setup. |
| src/test/java/com/researchspace/export/pdf/HTMLStringGeneratorTest.java | Refactors default stubbing into helper methods and removes lenient usage. |
| src/test/java/com/researchspace/dao/customliquibaseupdates/MySQLVersionPreconditionTest.java | Tightens DB connection stubbing to strict. |
| src/test/java/com/researchspace/auth/UserPermissionUtilsTest.java | Converts lenient stubbing helper to strict. |
| src/test/java/com/researchspace/auth/LdapRealmTest.java | Removes lenient stub for properties flag. |
| src/test/java/com/researchspace/api/v1/service/impl/ExportApiSpringBatchHandlerTest.java | Removes dead/late stubbing and makes success stubs centralized. |
| src/test/java/com/researchspace/api/v1/controller/SysadminApiControllerTest.java | Refactors deletion stubbing into helper; tightens exception assertions; removes lenient stubs. |
| src/test/java/com/researchspace/api/v1/controller/SampleTemplatesRevisionsEndpointTest.java | Converts lenient message stub to strict. |
| src/test/java/com/researchspace/api/v1/controller/InventoryVersionEndpointsNotFoundTest.java | Converts lenient message stub to strict. |
| src/test/java/com/researchspace/api/v1/controller/InstrumentTemplatesRevisionsEndpointTest.java | Converts lenient message stub to strict. |
| src/test/java/com/researchspace/api/v1/controller/FolderApiControllerTest.java | Removes lenient parent-folder stubs not required by tested paths. |
| src/test/java/com/researchspace/api/v1/controller/FilesApiControllerTest.java | Makes base URL stub explicit per-test. |
| src/test/java/com/researchspace/api/v1/controller/APIFileUploadThrottlingInterceptorTest.java | Converts lenient throttler stats stub to strict. |
| src/test/java/com/researchspace/api/v1/config/APIProdConfigTestBase.java | Renames BeforeClass method to skipFastRuns. |
| src/test/java/com/researchspace/api/v1/auth/CombinedApiAuthenticatorTest.java | Removes unused lenient stubbing from OAuth test path. |
| src/test/java/com/researchspace/admin/service/FileLocationBasedRetrieverTest.java | Refactors “ok properties” into factory method to avoid lenient mocking. |
| src/test/java/com/axiope/webapp/taglib/ViteClientTagTest.java | Replaces custom request/writer mocks with Spring mock web objects; removes lenient stubs. |
| src/test/java/com/axiope/webapp/taglib/CacheVersionTagTest.java | Replaces custom attribute maps with Spring mock web objects; removes lenient stubs. |
| src/test/java/com/axiope/webapp/taglib/BundleTagTest.java | Reworks taglib tests to use Spring mock web objects and explicit pageContext stubbing. |
| src/test/java/com/axiope/webapp/taglib/AssetUrlTagTest.java | Reworks taglib tests to use Spring mock web objects and explicit pageContext stubbing. |
| src/test/java/com/axiope/webapp/listener/StartupListenerViteProxyTest.java | Removes lenient strictness annotation on Mockito extension. |
| src/test/java/com/axiope/webapp/dev/ViteDevServerProxyServletTest.java | Removes lenient strictness annotation; refactors stubbing into helper for upstream success. |
| src/test/java/com/axiope/webapp/dev/ViteDevServerProxyServletMimeTypeTest.java | Adds focused unit test for MIME-type inference helper. |
Review details
- Files reviewed: 73/73 changed files
- Comments generated: 2
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
03e274c to
74233a8
Compare
74233a8 to
ed4c977
Compare
There was a problem hiding this comment.
🟢 Ready to approve
The changes are confined to test code and consistently replace lenient stubbing with explicit, per-test setup without introducing verified behavioral regressions.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
- Files reviewed: 73/73 changed files
- Comments generated: 0 new
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
7c11e8c to
3f04454
Compare
3f04454 to
72016b0
Compare
Description
Removes all non-strict uses of Mockito in order to increase the quality of tests.