added XML parsing test#2293
Conversation
|
Warning Rate limit exceeded@rrayst has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 17 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds parsing tests and refactors test compilation infrastructure: replaces the old CustomJavaFileManager with a logging in-memory manager, introduces multiple in-memory file/classloader utilities, changes CompilerHelper/CompilerResult APIs, and adds new test classes and supporting utilities for overlayed in-memory resources. Changes
Sequence Diagram(s)sequenceDiagram
participant Test
participant CompilerHelper
participant JavaCompiler
participant LoggingInMemoryJavaFileManager
participant InMemoryClassLoader
participant Parser
Test->>CompilerHelper: compile(sourceFiles, logOutput)
CompilerHelper->>LoggingInMemoryJavaFileManager: prepare (javaSources + resources)
CompilerHelper->>JavaCompiler: run compilation task with LoggingInMemoryJavaFileManager
JavaCompiler-->>CompilerHelper: CompilerResult(compilationSuccess, diagnostics, classLoader)
Test->>CompilerHelper: parse(CompilerResult, xmlSpringConfig)
CompilerHelper->>Parser: parse using CompositeClassLoader(InMemoryClassLoader, result.classLoader)
Parser->>InMemoryClassLoader: loadClass / getResource (overlay first)
Parser-->>Test: parsed Spring configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/ok-to-test |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
annot/src/test/java/com/predic8/membrane/annot/SpringConfigXSDErrorsTest.java (1)
20-21: Remove unused imports.The imports for
JavaFileObjectandListare no longer needed after replacing explicit type declarations withvarfor local variables. Clean up the imports.-import javax.tools.JavaFileObject; -import java.util.List;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
annot/src/test/java/com/predic8/membrane/annot/ParsingTest.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/SpringConfigXSDErrorsTest.java(17 hunks)annot/src/test/java/com/predic8/membrane/annot/SpringConfigurationXSDGeneratingAnnotationProcessorTest.java(2 hunks)annot/src/test/java/com/predic8/membrane/annot/util/CompilerHelper.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/CompilerResult.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/CompositeClassLoader.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/ConcatenatingEnumeration.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/CustomJavaFileManager.java(0 hunks)annot/src/test/java/com/predic8/membrane/annot/util/InMemoryClassLoader.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/InMemoryData.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/InMemoryFileObject.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/InMemoryJavaFileObject.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/InMemoryURLStreamHandler.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/InnerFileObject.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/LoggingFileObject.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/LoggingInMemoryJavaFileManager.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/LoggingJavaFileObject.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/OverlayInMemoryFile.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/OverlayInMemoryJavaFile.java(1 hunks)
💤 Files with no reviewable changes (1)
- annot/src/test/java/com/predic8/membrane/annot/util/CustomJavaFileManager.java
🧰 Additional context used
🧬 Code graph analysis (3)
annot/src/test/java/com/predic8/membrane/annot/ParsingTest.java (2)
annot/src/test/java/com/predic8/membrane/annot/util/CompilerHelper.java (1)
CompilerHelper(35-198)annot/src/test/java/com/predic8/membrane/annot/SpringConfigurationXSDGeneratingAnnotationProcessorTest.java (1)
SpringConfigurationXSDGeneratingAnnotationProcessorTest(23-66)
annot/src/test/java/com/predic8/membrane/annot/util/InMemoryClassLoader.java (1)
annot/src/test/java/com/predic8/membrane/annot/util/InMemoryURLStreamHandler.java (1)
InMemoryURLStreamHandler(14-53)
annot/src/test/java/com/predic8/membrane/annot/util/LoggingInMemoryJavaFileManager.java (1)
annot/src/test/java/com/predic8/membrane/annot/util/InMemoryURLStreamHandler.java (1)
InMemoryURLStreamHandler(14-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Automated tests
- GitHub Check: Analyze (java)
🔇 Additional comments (2)
annot/src/test/java/com/predic8/membrane/annot/SpringConfigXSDErrorsTest.java (1)
30-30: Consistent and appropriate use ofvarfor local variable type inference.The refactoring from explicit
List<JavaFileObject>andCompilerResulttypes tovaris clean and follows modern Java best practices. The inferred types are unambiguous from the method return types, making the code more readable without sacrificing clarity.Also applies to: 50-50, 66-66, 77-77, 100-100, 126-126, 142-142, 158-158, 174-174, 196-196, 215-215, 236-236, 258-258, 283-283, 298-298, 318-318, 341-341
annot/src/test/java/com/predic8/membrane/annot/ParsingTest.java (1)
44-78: Great coverage for nested child elements. The newchildElementscase thoroughly exercises the generated schema’s ability to handle polymorphic children, which boosts confidence in the annotation processor’s output.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
annot/src/test/java/com/predic8/membrane/annot/util/LoggingInMemoryJavaFileManager.java (1)
124-139: Allow resource snippets without payloads.The loop around Line 130 still throws when the snippet is only
resource /foo(no newline content). This is the same issue flagged earlier and it still breaks otherwise valid test inputs. Please tolerate a missing body by defaulting to""instead of raising.Apply this diff:
- while(true) { - parts = content.split("\n", 2); - if (parts.length != 2) - throw new RuntimeException("Invalid resource file: " + content + ". The resource is expected to have the format 'resource <path>\n<content>'."); + while (true) { + parts = content.split("\n", 2); + if (parts.length < 1) + throw new RuntimeException("Invalid resource file: " + content); if (!parts[0].isEmpty()) break; content = parts[1]; }; String name = parts[0].substring(9).trim(); - return new OverlayInMemoryFile(name, parts[1]); + String body = parts.length > 1 ? parts[1] : ""; + return new OverlayInMemoryFile(name, body);annot/src/test/java/com/predic8/membrane/annot/util/InMemoryURLStreamHandler.java (1)
44-51: Handle missing base resources safely.Line 50 still blindly wraps
data.content.get(uri)without verifying thatdataexists or that the map contains the key. When absent,new ByteArrayInputStream(null)throws aNullPointerException. Mirror the overlay guard: resolve the bytes first, throwFileNotFoundExceptionif null, then build the stream.Apply this diff:
- return new ByteArrayInputStream(data.content.get(uri)); + byte[] buffer = data != null ? data.content.get(uri) : null; + if (buffer == null) + throw new FileNotFoundException("No in-memory resource for " + uri); + return new ByteArrayInputStream(buffer);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
annot/src/test/java/com/predic8/membrane/annot/util/CompilerHelper.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/InMemoryClassLoader.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/InMemoryURLStreamHandler.java(1 hunks)annot/src/test/java/com/predic8/membrane/annot/util/LoggingInMemoryJavaFileManager.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- annot/src/test/java/com/predic8/membrane/annot/util/InMemoryClassLoader.java
🧰 Additional context used
🧬 Code graph analysis (1)
annot/src/test/java/com/predic8/membrane/annot/util/LoggingInMemoryJavaFileManager.java (1)
annot/src/test/java/com/predic8/membrane/annot/util/InMemoryURLStreamHandler.java (1)
InMemoryURLStreamHandler(15-58)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Automated tests
- GitHub Check: Analyze (java)
2 participants
Summary by CodeRabbit
Tests
Chores