Skip to content

Commit 7b5d640

Browse files
jabrenajabrena
committed
New progress
1 parent 87ced2d commit 7b5d640

4 files changed

Lines changed: 282 additions & 53 deletions

File tree

‎spml/src/main/resources/131-java-unit-testing.xml‎

Lines changed: 194 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ void testUserValidation() { // Tests multiple conditions at once
361361
<rule-section number="7" id="use-parameterized-tests">
362362
<rule-header>
363363
<rule-title>Use Parameterized Tests for Data Variations</rule-title>
364-
<rule-subtitle>Use @ParameterizedTest for testing the same logic with different inputs</rule-subtitle>
364+
<rule-subtitle>Use `@ParameterizedTest` for testing the same logic with different inputs</rule-subtitle>
365365
</rule-header>
366366
<rule-description>
367367
When testing a method's behavior across various input values or boundary conditions, leverage JUnit 5's parameterized tests (`@ParameterizedTest` with sources like `@ValueSource`, `@CsvSource`, `@MethodSource`). This avoids code duplication and clearly separates the test logic from the test data.
@@ -435,6 +435,29 @@ class RepetitiveCalculatorTest {
435435
<rule-description>
436436
Unit tests should focus solely on the logic of the class being tested (System Under Test - SUT), not its dependencies (database, network services, other classes). Use mocking frameworks like Mockito to create mock objects that simulate the behavior of these dependencies. This ensures tests are fast, reliable, and truly test the unit in isolation.
437437
</rule-description>
438+
<rule-notes>
439+
<notes-title>Key Mockito Concepts:</notes-title>
440+
<note-item>
441+
<note-term>`mock(Class&lt;T&gt; classToMock)`</note-term>
442+
<note-description>Creates a mock object of a given class or interface.</note-description>
443+
</note-item>
444+
<note-item>
445+
<note-term>`when(mock.methodCall()).thenReturn(value)`</note-term>
446+
<note-description>Defines the behavior of a mock object's method. When the specified method is called on the mock, it will return the defined `value`.</note-description>
447+
</note-item>
448+
<note-item>
449+
<note-term>`verify(mock).methodCall()`</note-term>
450+
<note-description>Verifies that a specific method was called on the mock object. You can also specify the number of times (`times(n)`), at least once (`atLeastOnce()`), etc.</note-description>
451+
</note-item>
452+
<note-item>
453+
<note-term>`@Mock` Annotation</note-term>
454+
<note-description>Used with `@ExtendWith(MockitoExtension.class)` (JUnit 5) to automatically create mocks for fields.</note-description>
455+
</note-item>
456+
<note-item>
457+
<note-term>`@InjectMocks` Annotation</note-term>
458+
<note-description>Creates an instance of the class under test and automatically injects fields annotated with `@Mock` into it.</note-description>
459+
</note-item>
460+
</rule-notes>
438461
<code-examples>
439462
<good-example>
440463
<code-block language="java"><![CDATA[import org.junit.jupiter.api.Test;
@@ -448,6 +471,8 @@ import java.util.Optional;
448471
import static org.assertj.core.api.Assertions.assertThat;
449472
import static org.mockito.Mockito.*; // Import static methods
450473
474+
// Assume classes: UserService, UserRepository, User
475+
451476
@ExtendWith(MockitoExtension.class) // Integrate Mockito with JUnit 5
452477
class UserServiceTest {
453478
@@ -474,6 +499,39 @@ class UserServiceTest {
474499
verify(userRepository, times(1)).findById("123");
475500
verifyNoMoreInteractions(userRepository); // Optional: ensure no other methods were called
476501
}
502+
503+
@Test
504+
@DisplayName("Should return empty optional when user not found")
505+
void findUserById_NotFound() {
506+
// Given
507+
// Define mock behavior: when findById is called with any string, return empty
508+
when(userRepository.findById(anyString())).thenReturn(Optional.empty());
509+
510+
// When
511+
Optional<User> actualUser = userService.findUserById("unknownId");
512+
513+
// Then
514+
assertThat(actualUser).isNotPresent();
515+
verify(userRepository).findById("unknownId"); // Verify the specific call
516+
}
517+
518+
@Test
519+
@DisplayName("Should save user successfully")
520+
void saveUser() {
521+
// Given
522+
User userToSave = new User(null, "Jane Doe"); // ID might be generated on save
523+
User savedUser = new User("genId", "Jane Doe");
524+
// Define behavior for save: return the user with an ID
525+
when(userRepository.save(any(User.class))).thenReturn(savedUser);
526+
527+
// When
528+
User result = userService.saveUser(userToSave);
529+
530+
// Then
531+
assertThat(result).isEqualTo(savedUser);
532+
// Verify that save was called with the correct user object (or use ArgumentCaptor for complex cases)
533+
verify(userRepository).save(userToSave);
534+
}
477535
}]]></code-block>
478536
</good-example>
479537
<bad-example>
@@ -505,7 +563,7 @@ class UserServiceTestBad {
505563
<rule-subtitle>Use code coverage as a guide, not a definitive quality metric</rule-subtitle>
506564
</rule-header>
507565
<rule-description>
508-
Tools like JaCoCo can measure which lines of code are executed by your tests (code coverage). Aiming for high coverage (e.g., &gt;80% line/branch coverage) is generally good practice, as it indicates most code paths are tested. However, 100% coverage doesn't guarantee bug-free code or high-quality tests. Focus on writing meaningful tests for critical logic and edge cases rather than solely chasing coverage numbers.
566+
Tools like JaCoCo can measure which lines of code are executed by your tests (code coverage). Aiming for high coverage (e.g., &gt;80% line/branch coverage) is generally good practice, as it indicates most code paths are tested. However, 100% coverage doesn't guarantee bug-free code or high-quality tests. Focus on writing meaningful tests for critical logic and edge cases rather than solely chasing coverage numbers. A test might cover a line but not actually verify its correctness effectively.
509567
</rule-description>
510568
</rule-section>
511569

@@ -535,7 +593,7 @@ class UserServiceTestBad {
535593
<rule-subtitle>Avoid common testing anti-patterns</rule-subtitle>
536594
</rule-header>
537595
<rule-description>
538-
Testing Implementation Details: Avoid testing implementation details that might change, leading to brittle tests. Focus on testing behavior and outcomes. Hard-coded Values: Avoid hard-coding values in tests. Use constants or test data to make tests more maintainable. Complex Test Logic: Keep test logic simple and avoid complex calculations or conditional statements within tests.
596+
Testing Implementation Details: Avoid testing implementation details that might change, leading to brittle tests. Focus on testing behavior and outcomes. Hard-coded Values: Avoid hard-coding values in tests. Use constants or test data to make tests more maintainable. Complex Test Logic: Keep test logic simple and avoid complex calculations or conditional statements within tests. Ignoring Edge Cases: Don't ignore edge cases or boundary conditions. Ensure tests cover a wide range of inputs, including invalid or unexpected values. Slow Tests: Avoid slow tests that discourage developers from running them frequently. Over-reliance on Mocks: Mock judiciously; too many mocks can obscure the actual behavior and make tests less reliable. Ignoring Test Failures: Never ignore failing tests. Investigate and fix them promptly.
539597
</rule-description>
540598
</rule-section>
541599

@@ -545,7 +603,9 @@ class UserServiceTestBad {
545603
<rule-subtitle>Ensure proper state isolation between tests</rule-subtitle>
546604
</rule-header>
547605
<rule-description>
548-
Isolated State: Ensure each test has its own isolated state to avoid interference between tests. Use @BeforeEach to reset the state before each test. Immutable Objects: Prefer immutable objects to simplify state management and avoid unexpected side effects. Stateless Components: Design stateless components whenever possible to reduce the need for state management in tests.
606+
- **Isolated State:** Ensure each test has its own isolated state to avoid interference between tests. Use `@BeforeEach` to reset the state before each test.
607+
- **Immutable Objects:** Prefer immutable objects to simplify state management and avoid unexpected side effects.
608+
- **Stateless Components:** Design stateless components whenever possible to reduce the need for state management in tests.
549609
</rule-description>
550610
</rule-section>
551611

@@ -555,7 +615,9 @@ class UserServiceTestBad {
555615
<rule-subtitle>Test error conditions and exception handling</rule-subtitle>
556616
</rule-header>
557617
<rule-description>
558-
Expected Exceptions: Use AssertJ's assertThatThrownBy to verify that a method throws the expected exception under specific conditions. Exception Messages: Assert the exception message to ensure the correct error is being thrown with helpful context. Graceful Degradation: Test how the application handles errors and gracefully degrades when dependencies are unavailable.
618+
- **Expected Exceptions:** Use AssertJ's `assertThatThrownBy` to verify that a method throws the expected exception under specific conditions.
619+
- **Exception Messages:** Assert the exception message to ensure the correct error is being thrown with helpful context.
620+
- **Graceful Degradation:** Test how the application handles errors and gracefully degrades when dependencies are unavailable.
559621
</rule-description>
560622
</rule-section>
561623

@@ -565,8 +627,133 @@ class UserServiceTestBad {
565627
<rule-subtitle>Utilize JSpecify annotations for explicit nullness contracts</rule-subtitle>
566628
</rule-header>
567629
<rule-description>
568-
Employ JSpecify annotations (org.jspecify.annotations.*) such as @NullMarked, @Nullable, and @NonNull to clearly define the nullness expectations of method parameters, return types, and fields within your tests and the code under test. This practice enhances code clarity, enables static analysis tools to catch potential NullPointerExceptions early, and improves the overall robustness of your tests and application code.
630+
Employ JSpecify annotations (org.jspecify.annotations.*) such as @NullMarked, @Nullable, and @NonNull to clearly define the nullness expectations of method parameters, return types, and fields within your tests and the code under test. This practice enhances code clarity, enables static analysis tools to catch potential NullPointerExceptions early, and improves the overall robustness of your tests and application code. In test code, this is particularly important for defining the expected behavior of mocks and verifying interactions with potentially null values.
569631
</rule-description>
632+
<code-examples>
633+
<good-example>
634+
<code-block language="java"><![CDATA[import org.jspecify.annotations.NullMarked;
635+
import org.jspecify.annotations.Nullable;
636+
import org.junit.jupiter.api.Test;
637+
import org.junit.jupiter.api.extension.ExtendWith;
638+
import org.mockito.Mock;
639+
import org.mockito.junit.jupiter.MockitoExtension;
640+
641+
import static org.assertj.core.api.Assertions.assertThat;
642+
import static org.mockito.Mockito.when;
643+
644+
// Assume:
645+
// @NullMarked // Usually at package-info.java or a higher-level class
646+
// interface DataService {
647+
// @Nullable String getData(String key);
648+
// String processData(@Nullable String data);
649+
// }
650+
//
651+
// class MyProcessor {
652+
// private final DataService dataService;
653+
//
654+
// MyProcessor(DataService dataService) {
655+
// this.dataService = dataService;
656+
// }
657+
//
658+
// String execute(String key) {
659+
// @Nullable String rawData = dataService.getData(key);
660+
// // rawData could be null here, static analysis would warn if not checked
661+
// if (rawData == null) {
662+
// return dataService.processData(null);
663+
// }
664+
// return dataService.processData(rawData.toUpperCase());
665+
// }
666+
// }
667+
668+
669+
@NullMarked // Applies non-null by default to this test class
670+
@ExtendWith(MockitoExtension.class)
671+
class MyProcessorTest {
672+
673+
@Mock
674+
private DataService mockDataService;
675+
676+
private MyProcessor myProcessor;
677+
678+
@Test
679+
void should_handleNullData_when_serviceReturnsNull() {
680+
// Given
681+
myProcessor = new MyProcessor(mockDataService);
682+
String key = "testKey";
683+
// JSpecify helps clarify that getData can return null
684+
when(mockDataService.getData(key)).thenReturn(null);
685+
// JSpecify helps clarify that processData can accept null
686+
when(mockDataService.processData(null)).thenReturn("processed:null");
687+
688+
689+
// When
690+
String result = myProcessor.execute(key);
691+
692+
// Then
693+
assertThat(result).isEqualTo("processed:null");
694+
}
695+
696+
@Test
697+
void should_processNonNullData_when_serviceReturnsData() {
698+
// Given
699+
myProcessor = new MyProcessor(mockDataService);
700+
String key = "testKey";
701+
String serviceData = "someData"; // Effectively @NonNull due to @NullMarked context
702+
// getData's return is @Nullable, but we are testing the non-null path
703+
when(mockDataService.getData(key)).thenReturn(serviceData);
704+
// processData's argument is @Nullable, here we pass a non-null value
705+
when(mockDataService.processData("SOMEDATA")).thenReturn("processed:SOMEDATA");
706+
707+
// When
708+
String result = myProcessor.execute(key);
709+
710+
// Then
711+
assertThat(result).isEqualTo("processed:SOMEDATA");
712+
}
713+
}]]></code-block>
714+
</good-example>
715+
<bad-example>
716+
<code-block language="java"><![CDATA[// No JSpecify annotations, nullness is ambiguous
717+
// class DataService {
718+
// String getData(String key); // Is null return possible? Is key nullable?
719+
// String processData(String data); // Can data be null?
720+
// }
721+
//
722+
// class MyProcessor {
723+
// // ... constructor ...
724+
// String execute(String key) {
725+
// String rawData = dataService.getData(key);
726+
// // Potential NPE here if getData can return null and it's not handled.
727+
// // The contract is unclear without annotations.
728+
// return dataService.processData(rawData.toUpperCase());
729+
// }
730+
// }
731+
732+
class MyProcessorTest {
733+
// ... test setup ...
734+
735+
@Test
736+
void testProcessing() {
737+
// Given
738+
MyProcessor myProcessor = new MyProcessor(mockDataService);
739+
String key = "testKey";
740+
// Ambiguity: if getData returns null, this test might pass or fail unexpectedly
741+
// depending on mock setup, but the underlying contract isn't clear.
742+
when(mockDataService.getData(key)).thenReturn("someData");
743+
when(mockDataService.processData("SOMEDATA")).thenReturn("processed:SOMEDATA");
744+
// If getData could return null and mockDataService.processData isn't
745+
// prepared for mockDataService.processData(null), an NPE could occur
746+
// in the code under test or the test itself, masking the real issue.
747+
748+
// When
749+
String result = myProcessor.execute(key);
750+
751+
// Then
752+
assertThat(result).isEqualTo("processed:SOMEDATA");
753+
}
754+
}]]></code-block>
755+
</bad-example>
756+
</code-examples>
570757
</rule-section>
571758

572759
<rule-section number="16" id="right-bicep-questions">
@@ -934,4 +1121,4 @@ public class UserValidationPoorTest {
9341121
</code-examples>
9351122
</rule-section>
9361123
</content-sections>
937-
</system-prompt>
1124+
</system-prompt>

‎spml/src/main/resources/system-prompt.xsd‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@
136136
<xs:sequence>
137137
<xs:element ref="rule-header"/>
138138
<xs:element ref="rule-description"/>
139+
<xs:element ref="rule-notes" minOccurs="0" maxOccurs="unbounded"/>
139140
<xs:element ref="code-examples" minOccurs="0"/>
140141
</xs:sequence>
141142
<xs:attribute name="number" type="xs:string" use="optional"/>
@@ -164,6 +165,30 @@
164165
</xs:complexType>
165166
</xs:element>
166167

168+
<!-- Rule notes for additional explanatory content -->
169+
<xs:element name="rule-notes">
170+
<xs:complexType>
171+
<xs:sequence>
172+
<xs:element ref="notes-title" minOccurs="0"/>
173+
<xs:element ref="note-item" minOccurs="0" maxOccurs="unbounded"/>
174+
</xs:sequence>
175+
</xs:complexType>
176+
</xs:element>
177+
178+
<xs:element name="notes-title" type="xs:string"/>
179+
180+
<xs:element name="note-item">
181+
<xs:complexType>
182+
<xs:sequence>
183+
<xs:element ref="note-term"/>
184+
<xs:element ref="note-description"/>
185+
</xs:sequence>
186+
</xs:complexType>
187+
</xs:element>
188+
189+
<xs:element name="note-term" type="xs:string"/>
190+
<xs:element name="note-description" type="xs:string"/>
191+
167192
<!-- Code examples with good and bad patterns -->
168193
<xs:element name="code-examples">
169194
<xs:complexType>

‎spml/src/main/resources/unified-generator.xsl‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,25 @@ Role definition: </xsl:text><xsl:value-of select="normalize-space(system-charact
8686

8787
Title: </xsl:text><xsl:value-of select="normalize-space(rule-header/rule-subtitle)"/>
8888
<xsl:text>
89-
Description: </xsl:text><xsl:value-of select="normalize-space(rule-description)"/>
89+
Description: </xsl:text> <xsl:value-of select="normalize-space(rule-description)"/>
90+
91+
<xsl:for-each select="rule-notes">
92+
<xsl:text>
93+
94+
</xsl:text>
95+
<xsl:if test="notes-title">
96+
<xsl:text>**</xsl:text><xsl:value-of select="normalize-space(notes-title)"/>
97+
<xsl:text>**
98+
99+
</xsl:text>
100+
</xsl:if>
101+
<xsl:for-each select="note-item">
102+
<xsl:text>* **</xsl:text><xsl:value-of select="normalize-space(note-term)"/>
103+
<xsl:text>**: </xsl:text><xsl:value-of select="normalize-space(note-description)"/>
104+
<xsl:text>
105+
</xsl:text>
106+
</xsl:for-each>
107+
</xsl:for-each>
90108

91109
<xsl:if test="code-examples/good-example">
92110
<xsl:text>

0 commit comments

Comments
 (0)