JavadocMetadataScrapperTest and InlineConfigParser fixes #10827
Conversation
| @@ -213,10 +227,7 @@ else if (ast.getType() == JavadocTokenTypes.SINCE_LITERAL) { | |||
| public void finishJavadocTree(DetailNode rootAst) { | |||
| moduleDetails.setDescription(getDescriptionText()); | |||
| if (isTopLevelClassJavadoc()) { | |||
| if (getFileContents().getFileName().contains("test")) { | |||
There was a problem hiding this comment.
This is the main reason the property is needed to drop having this code that checks the file name which will cause issues with checks with that word in them. New property always defaults to write.
| @@ -603,13 +617,18 @@ private static String getPackageName(String filePath) { | |||
| return Collections.unmodifiableMap(MODULE_DETAILS_STORE); | |||
| } | |||
|
|
|||
| /** Reset the module detail store of any previous information. */ | |||
| public static void resetModuleDetailsStore() { | |||
| MODULE_DETAILS_STORE.clear(); | |||
There was a problem hiding this comment.
Tests are writing to a global static field. Nothing ensure this field is cleared, so this reset method is there to ensure the field is cleared.
| @@ -234,7 +248,7 @@ public void finishJavadocTree(DetailNode rootAst) { | |||
| * | |||
| * @param ast javadoc ast | |||
| */ | |||
| public void scrapeContent(DetailNode ast) { | |||
| private void scrapeContent(DetailNode ast) { | |||
There was a problem hiding this comment.
No reason this method should be public.
| /** | ||
| * Check if the current javadoc block comment AST corresponds to the top-level class as we | ||
| * only want to scrape top-level class javadoc. | ||
| * | ||
| * @return true if the current AST corresponds to top level class | ||
| */ | ||
| public boolean isTopLevelClassJavadoc() { | ||
| private boolean isTopLevelClassJavadoc() { |
| } | ||
|
|
||
| @Test | ||
| public void testPropertyNameWithNoCodeTag() { |
There was a problem hiding this comment.
Removed test basically got encompassed into the new tests.
|
|
||
| assertNull(testCheckMeta.getProperties().get(0).getName(), | ||
| "Check with no property name {@code } tag should have null property name"); | ||
| private static String split(String string, int size) { |
There was a problem hiding this comment.
Split code is to help ensure line does not exceed the 100 character limit.
rnveach
changed the title
| @@ -76,23 +78,18 @@ public static void generate(String... args) throws IOException, CheckstyleExcept | |||
| private static void dumpMetadata(Checker checker, String path) throws CheckstyleException, | |||
| IOException { | |||
| final List<File> validFiles = new ArrayList<>(); | |||
| if (path.endsWith(".java")) { | |||
| validFiles.add(new File(path)); | |||
There was a problem hiding this comment.
Removed testing single files which I assume was only used by UTs. Most of our needs scan folders.
reworks JavadocMetadataScrapperTest to make more sense
Moved all tests related to
JavadocMetadataScrapperintoJavadocMetadataScrapperTestand use BDD and actually verify outputs from the check. Hopefully, at some point we can use pitest and such on this class to verify it's code.changes InlineConfigParser to respect fully qualified name
This is a simple change to so that when it gets the fully qualified name, it takes as the fully qualified name and doesn't make any other changes. This is all because
JavadocMetadataScrapperdoesn't end withCheckso BDD can't recognize it. No idea why the class was named this, but BDD should still be changed as I expect the FQN to be the FQN.