-
Notifications
You must be signed in to change notification settings - Fork 80
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
TestsShouldNotBePublic
should reduce visibility if used elsewhere in Javadoc links
#458
Comments
Oh wow; at first I thought this was a variant of the existing issue related to extended classes: But in this case it seems the classes are only ever used for |
TestsShouldNotBePublic
should reduce visibility if used elsewhere in Javadoc links
Not all: org.jabref.logic.exporter.BibtexDatabaseWriterTest#roundtripWin1252HeaderKept Path testFile = Path.of(BibtexImporterTest.class.getResource("encoding-windows-1252-with-header.bib").toURI()); And other similar things. I rewrote them by duplicating the test files and changing the access. In the case of JavaDocs, the class reference should be changed to a fully qualified class referernce. |
I have an edge case here: interface SearchBasedFetcherCapabilityTest {
...
@Test
default void supportsYearSearch() throws Exception {
...
}
...
class SpringerFetcherTest implements SearchBasedFetcherCapabilityTest, PagedSearchFetcherTest {
...
@Test
@Disabled("401 as of 2024-08-18")
@Override
void supportsJournalSearch() {
}
} I think, I cannot easily rewrite that. We decided to have tests implementing interfaces ensuring consistency on compiler level and not on checker or manual review level. |
I think it should be ready to skip methods annotated with override. And we already have a recipe that adds missing override annotations. Could be worth adding that cheap check here. Would you be up for contributing that to the project? Happy to help guide of course. |
I've explore overrides a bit in 671fe03 but saw no issue there. Reading your comment again I'm not sure if there even was an issue with the recipe. Could you clarify a bit what you're after? |
I applied
TestsShouldNotBePublic
to JabRef's code.I get compile errors afterwards:
/home/runner/work/jabref/jabref/src/test/java/org/jabref/logic/bst/BstFunctionsTest.java:7: error: BstCaseChangersTest is not public in org.jabref.logic.bst.util; cannot be accessed from outside package
import org.jabref.logic.bst.util.BstCaseChangersTest;
I know, this is bad software design. However, I think, rewriting should not introduce compile errors.
Refs JabRef/jabref#10797
The text was updated successfully, but these errors were encountered: