这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@dhrubo55
Copy link
Contributor

@dhrubo55 dhrubo55 commented Mar 3, 2025

No description provided.

}

@Test
void testCompileFromString_Success() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use given_when_then or when_then BDD style syntax for test names (same elsewhere)

@dhrubo55 dhrubo55 force-pushed the BAEL-8962-Check-if-a-code-compiles-in-Java-using-JavaCompiler branch from e173bdc to e63c994 Compare March 16, 2025 11:00


JavaCompiler.CompilationTask task = compiler.getTask(
null, // Writer for compiler output
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
null, // Writer for compiler output
null, // Writer for compiler output

return success;
}

// Add this method to JavaCompilerUtils
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

private boolean compile(Iterable<? extends JavaFileObject> compilationUnits) {
DiagnosticCollector<JavaFileObject> diagnostics = new DiagnosticCollector<>();


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra line

loadedClass.getMethod("main", String[].class).invoke(null, (Object) args);
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra line

}

@Test
void given_simpleHelloWorldClass_when_compiledFromString_then_compilationSucceeds() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the right syntax, see the suggestion for the syntax we require

Suggested change
void given_simpleHelloWorldClass_when_compiledFromString_then_compilationSucceeds() {
void givenSimpleHelloWorldClass_whenCompiledFromString_thenCompilationSucceeds() {


@Test
void given_simpleHelloWorldClass_when_compiledFromString_then_compilationSucceeds() {
// Simple "Hello World" class
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove all the comments from the tests, the assertion messages and test layout should speak for itself

<dependency>
<groupId>com.github.stefanbirkner</groupId>
<artifactId>system-lambda</artifactId>
<version>1.2.1</version>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract a version number property (this is another mandatory guideline)

DiagnosticCollector<JavaFileObject> diagnostics = new DiagnosticCollector<>();

JavaCompiler.CompilationTask task = compiler.getTask(
null, // Writer for compiler output
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alignment is still not correct here. Please review it visually and fix it.

Comment on lines 15 to 18
/**
* A utility class for compiling Java source code using the Java Compiler API.
* This class provides methods to compile Java code from strings or files.
*/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also advised by the team that we have guidelines to not use comments to add explanations for the code. All the required explanations should be in the text. Can you please remove the comments and JavaDoc from the PR to address this, and move any comments that are important to note into the article text instead.

- rename test file to have UnitTest word at the end
- rename test file to have UnitTest word at the end
@lor6 lor6 merged commit 1dabe9d into eugenp:master May 5, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants