Skip to content

Bael 8585 #18643

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Bael 8585 #18643

wants to merge 5 commits into from

Conversation

3ga01
Copy link
Contributor

@3ga01 3ga01 commented Jun 24, 2025

No description provided.

3ga01 added 5 commits June 24, 2025 22:49
: updated pom

Signed-off-by: Emmanuel Mireku Omari <[email protected]>
Signed-off-by: Emmanuel Mireku Omari <[email protected]>
Signed-off-by: Emmanuel Mireku Omari <[email protected]>
Signed-off-by: Emmanuel Mireku Omari <[email protected]>
Comment on lines +136 to +164
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
<version>1.18.32</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<version>3.24.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.glassfish.expressly</groupId>
<artifactId>expressly</artifactId>
<version>5.0.0</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>2.18.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.jayway.jsonpath</groupId>
<artifactId>json-path</artifactId>
<version>2.8.0</version>
<scope>test</scope>
</dependency>
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a convention to extract properties for version numbers. Also check if any of these versions are already specified in the parent pom (e.g. AssertJ might be, we use it in lots of articles)

import lombok.AllArgsConstructor;
import lombok.Data;

@Data
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a convention to avoid using Lombok except on articles that explicitly cover it - please remove the Lombok dependency for that reason and use plain Java for this

}

@Test
void whenPasswordIsValid_thenNoViolations() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Across all the unit tests there is the same minor BDD naming convention issue.

BDD syntax has 3 parts:

  1. given (the preconditions about the test scenario)
  2. when (the action under test)
  3. then (the expected outcome)

In this example, instead of whenPasswordIsValid_thenNoViolations the convention is supposed to be givenPasswordIsValid_whenValidate_thenNoViolations

private ObjectMapper objectMapper;

@BeforeEach
void setup() {
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
void setup() {
void setUp() {

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.

2 participants