CU-86b1uerqp: Add enum with all possible MT sequence paths #208
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe pull request introduces a new enumeration, Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnum.javais excluded by!**/generated/**
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnumTest.java(1 hunks)
🧰 Additional context used
🪛 Markdownlint
CHANGELOG.md
6-6: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
🔇 Additional comments (2)
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnumTest.java (2)
1-8: LGTM! Clean and minimal imports.
The imports are well-organized and follow best practices with static imports for assertions.
9-10: LGTM! Appropriate test class declaration.
The class follows JUnit conventions with package-private visibility.
| @ParameterizedTest | ||
| @CsvSource({"MTMT101, MTMT101", "MTMT102, MTMT102", "MTMT508, MTMT508"}) | ||
| void testFromFieldNameValid(String fieldName, MtSequenceInfoEnum expectedEnum) { | ||
| assertEquals(expectedEnum, MtSequenceInfoEnum.valueOf(fieldName)); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add more test cases for valueOf method.
Consider adding test cases for:
- Different MT number formats
- Case sensitivity validation
- Boundary values
@CsvSource({
"MTMT101, MTMT101",
"MTMT102, MTMT102",
- "MTMT508, MTMT508"
+ "MTMT508, MTMT508",
+ "mtmt101, MTMT101", // Test case insensitivity if supported
+ "MTMT999, MTMT999" // Test boundary MT number
})Committable suggestion skipped: line range outside the PR's diff.
| @Test | ||
| void testFromFieldNameInvalid() { | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("INVALID")); | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("")); | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT")); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider additional invalid test cases.
The current error cases are good, but consider adding:
- Null input test
- Special characters
- Invalid MT numbers
@Test
void testFromFieldNameInvalid() {
assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("INVALID"));
assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf(""));
assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT"));
+ assertThrows(NullPointerException.class, () -> MtSequenceInfoEnum.valueOf(null));
+ assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MTMT@#$"));
+ assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MTMT000"));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test | |
| void testFromFieldNameInvalid() { | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("INVALID")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT")); | |
| } | |
| @Test | |
| void testFromFieldNameInvalid() { | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("INVALID")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT")); | |
| assertThrows(NullPointerException.class, () -> MtSequenceInfoEnum.valueOf(null)); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MTMT@#$")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MTMT000")); | |
| } |
| @ParameterizedTest | ||
| @CsvSource({ | ||
| "MTMT101, 'A,B'", | ||
| "MTMT102, 'A,B,C'", | ||
| "MTMT300, 'A,B,B/B1,B/B2,C,D,D/D1,E,E/E1,E/E1/E1a,E/E1/E1a/E1a1,F'", | ||
| "MTMT508, 'A,A/A1,B,B/B1,C'" | ||
| }) | ||
| void testGetSequencesInfo(MtSequenceInfoEnum sequenceInfo, String expectedSequences) { | ||
| assertEquals(expectedSequences, sequenceInfo.getSequencesInfo()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider enhancing test coverage.
While the current test cases are good, consider adding:
- Edge cases with special characters in sequence names
- Validation for sequence format correctness
- More MT types to ensure comprehensive coverage
Example additions:
@CsvSource({
"MTMT101, 'A,B'",
"MTMT102, 'A,B,C'",
"MTMT300, 'A,B,B/B1,B/B2,C,D,D/D1,E,E/E1,E/E1/E1a,E/E1/E1a/E1a1,F'",
- "MTMT508, 'A,A/A1,B,B/B1,C'"
+ "MTMT508, 'A,A/A1,B,B/B1,C'",
+ "MTMT199, ''", // Test empty sequence
+ "MTMT202, 'A'", // Test single sequence
+ "MTMT320, 'A/A1/A1a/A1a1/A1a1a'" // Test deeply nested sequence
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @ParameterizedTest | |
| @CsvSource({ | |
| "MTMT101, 'A,B'", | |
| "MTMT102, 'A,B,C'", | |
| "MTMT300, 'A,B,B/B1,B/B2,C,D,D/D1,E,E/E1,E/E1/E1a,E/E1/E1a/E1a1,F'", | |
| "MTMT508, 'A,A/A1,B,B/B1,C'" | |
| }) | |
| void testGetSequencesInfo(MtSequenceInfoEnum sequenceInfo, String expectedSequences) { | |
| assertEquals(expectedSequences, sequenceInfo.getSequencesInfo()); | |
| } | |
| @ParameterizedTest | |
| @CsvSource({ | |
| "MTMT101, 'A,B'", | |
| "MTMT102, 'A,B,C'", | |
| "MTMT300, 'A,B,B/B1,B/B2,C,D,D/D1,E,E/E1,E/E1/E1a,E/E1/E1a/E1a1,F'", | |
| "MTMT508, 'A,A/A1,B,B/B1,C'", | |
| "MTMT199, ''", // Test empty sequence | |
| "MTMT202, 'A'", // Test single sequence | |
| "MTMT320, 'A/A1/A1a/A1a1/A1a1a'" // Test deeply nested sequence | |
| }) | |
| void testGetSequencesInfo(MtSequenceInfoEnum sequenceInfo, String expectedSequences) { | |
| assertEquals(expectedSequences, sequenceInfo.getSequencesInfo()); | |
| } |
| @Test | ||
| void testAllEnumValuesHaveSequences() { | ||
| for (MtSequenceInfoEnum sequenceInfo : MtSequenceInfoEnum.values()) { | ||
| assertNotNull(sequenceInfo.getSequencesInfo(), "Sequence info should not be null"); | ||
| assertFalse(sequenceInfo.getSequencesInfo().isEmpty(), "Sequence info should not be empty"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enhance sequence validation checks.
While basic null and empty checks are good, consider adding:
- Format validation for sequences (e.g., valid separator usage)
- Sequence order validation
- Duplicate sequence validation
@Test
void testAllEnumValuesHaveSequences() {
for (MtSequenceInfoEnum sequenceInfo : MtSequenceInfoEnum.values()) {
assertNotNull(sequenceInfo.getSequencesInfo(), "Sequence info should not be null");
assertFalse(sequenceInfo.getSequencesInfo().isEmpty(), "Sequence info should not be empty");
+ // Validate sequence format
+ String[] sequences = sequenceInfo.getSequencesInfo().split(",");
+ for (String sequence : sequences) {
+ assertTrue(sequence.matches("[A-Z][0-9]?(/[A-Z][0-9]?)*"),
+ "Sequence " + sequence + " should match expected format");
+ }
+ // Check for duplicates
+ assertEquals(sequences.length, Arrays.stream(sequences).distinct().count(),
+ "Sequences should not contain duplicates");
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test | |
| void testAllEnumValuesHaveSequences() { | |
| for (MtSequenceInfoEnum sequenceInfo : MtSequenceInfoEnum.values()) { | |
| assertNotNull(sequenceInfo.getSequencesInfo(), "Sequence info should not be null"); | |
| assertFalse(sequenceInfo.getSequencesInfo().isEmpty(), "Sequence info should not be empty"); | |
| } | |
| } | |
| @Test | |
| void testAllEnumValuesHaveSequences() { | |
| for (MtSequenceInfoEnum sequenceInfo : MtSequenceInfoEnum.values()) { | |
| assertNotNull(sequenceInfo.getSequencesInfo(), "Sequence info should not be null"); | |
| assertFalse(sequenceInfo.getSequencesInfo().isEmpty(), "Sequence info should not be empty"); | |
| // Validate sequence format | |
| String[] sequences = sequenceInfo.getSequencesInfo().split(","); | |
| for (String sequence : sequences) { | |
| assertTrue(sequence.matches("[A-Z][0-9]?(/[A-Z][0-9]?)*"), | |
| "Sequence " + sequence + " should match expected format"); | |
| } | |
| // Check for duplicates | |
| assertEquals(sequences.length, Arrays.stream(sequences).distinct().count(), | |
| "Sequences should not contain duplicates"); | |
| } | |
| } |
src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnum.java
Outdated
Show resolved
Hide resolved
src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnum.java
Outdated
Show resolved
Hide resolved
src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnum.java
Outdated
Show resolved
Hide resolved
src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnum.java
Outdated
Show resolved
Hide resolved
ptorres-prowide
force-pushed
the
CU-86b1uerqp_Generar-un-enum-de-MTs-para-validar-sus-secuencias-y-paths
branch
from
a976493
to
0f675b6
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnum.javais excluded by!**/generated/**
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnumTest.java(1 hunks)
🧰 Additional context used
🪛 Markdownlint
CHANGELOG.md
6-6: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
🔇 Additional comments (1)
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnumTest.java (1)
35-40: 🧹 Nitpick (assertive)
Add more error test cases.
The error testing could be more comprehensive. Consider adding:
- Null input test
- Special characters in input
- Invalid MT numbers
- Extremely long inputs
@Test
void testFromFieldNameInvalid() {
assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("INVALID"));
assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf(""));
assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT"));
+ assertThrows(NullPointerException.class, () -> MtSequenceInfoEnum.valueOf(null));
+ assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT@101"));
+ assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT00A"));
+ assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT" + "1".repeat(1000)));
}Likely invalid or redundant comment.
| @ParameterizedTest | ||
| @CsvSource({ | ||
| "MT101, 'A,B'", | ||
| "MT102, 'A,B,C'", | ||
| "MT300, 'A,B,B/B1,B/B2,C,D,D/D1,E,E/E1,E/E1/E1a,E/E1/E1a/E1a1,F'", | ||
| "MT508, 'A,A/A1,B,B/B1,C'" | ||
| }) | ||
| void testGetSequencesInfo(MtSequenceInfoEnum sequenceInfo, String expectedSequences) { | ||
| assertEquals(toSet(expectedSequences), sequenceInfo.sequencesInfo()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider preserving sequence order in tests.
Converting sequences to Set loses the order information, which might be important for MT message validation. Consider:
- Using List instead of Set to preserve sequence order
- Adding more MT types for comprehensive coverage
- Testing edge cases (empty sequences, single sequence)
- void testGetSequencesInfo(MtSequenceInfoEnum sequenceInfo, String expectedSequences) {
- assertEquals(toSet(expectedSequences), sequenceInfo.sequencesInfo());
+ void testGetSequencesInfo(MtSequenceInfoEnum sequenceInfo, String expectedSequences) {
+ assertEquals(
+ Arrays.asList(expectedSequences.split(",")),
+ new ArrayList<>(sequenceInfo.sequencesInfo())
+ );
}Committable suggestion skipped: line range outside the PR's diff.
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnumTest.java
Outdated
Show resolved
Hide resolved
| @Test | ||
| void testAllEnumValuesHaveSequences() { | ||
| for (MtSequenceInfoEnum sequenceInfo : MtSequenceInfoEnum.values()) { | ||
| assertNotNull(sequenceInfo.sequencesInfo(), "Sequence info should not be null"); | ||
| assertFalse(sequenceInfo.sequencesInfo().isEmpty(), "Sequence info should not be empty"); | ||
| // Validate sequence format | ||
| Set<String> sequences = sequenceInfo.sequencesInfo(); | ||
| for (String sequence : sequences) { | ||
| assertTrue( | ||
| sequence.matches("^[A-Z](?:/[A-Z][0-9]?(?:[a-z][0-9]?[A-Za-z0-9]*)?)*$"), | ||
| "Sequence " + sequence + " should match expected format"); | ||
| } | ||
| // Check for duplicates | ||
| assertEquals( | ||
| sequences.size(), | ||
| Arrays.stream(sequences.toArray()).distinct().count(), | ||
| "Sequences should not contain duplicates"); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Document regex pattern and enhance enum completeness check.
The test is thorough but could be improved:
- Add documentation for the regex pattern explaining the format
- Simplify the duplicate check using Set
- Add verification that all known MT types are present
@Test
void testAllEnumValuesHaveSequences() {
+ // Known MT types that must be present
+ Set<String> requiredMTs = Set.of("MT101", "MT102", "MT300", "MT508");
+ Set<String> actualMTs = Arrays.stream(MtSequenceInfoEnum.values())
+ .map(Enum::name)
+ .collect(Collectors.toSet());
+ assertTrue(actualMTs.containsAll(requiredMTs),
+ "Missing required MT types: " +
+ requiredMTs.stream()
+ .filter(mt -> !actualMTs.contains(mt))
+ .collect(Collectors.joining(", ")));
+
for (MtSequenceInfoEnum sequenceInfo : MtSequenceInfoEnum.values()) {
assertNotNull(sequenceInfo.sequencesInfo(), "Sequence info should not be null");
assertFalse(sequenceInfo.sequencesInfo().isEmpty(), "Sequence info should not be empty");
- // Validate sequence format
+ // Validate sequence format:
+ // ^[A-Z] - Starts with uppercase letter
+ // (?:/[A-Z][0-9]? - Optional parts starting with / followed by letter and optional number
+ // (?:[a-z][0-9]? - Optional lowercase letter and number
+ // [A-Za-z0-9]*)?)*$ - Optional additional alphanumeric characters
Set<String> sequences = sequenceInfo.sequencesInfo();
for (String sequence : sequences) {
assertTrue(
sequence.matches("^[A-Z](?:/[A-Z][0-9]?(?:[a-z][0-9]?[A-Za-z0-9]*)?)*$"),
"Sequence " + sequence + " should match expected format");
}
- // Check for duplicates
- assertEquals(
- sequences.size(),
- Arrays.stream(sequences.toArray()).distinct().count(),
- "Sequences should not contain duplicates");
+ // No need for duplicate check as sequences is already a Set
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test | |
| void testAllEnumValuesHaveSequences() { | |
| for (MtSequenceInfoEnum sequenceInfo : MtSequenceInfoEnum.values()) { | |
| assertNotNull(sequenceInfo.sequencesInfo(), "Sequence info should not be null"); | |
| assertFalse(sequenceInfo.sequencesInfo().isEmpty(), "Sequence info should not be empty"); | |
| // Validate sequence format | |
| Set<String> sequences = sequenceInfo.sequencesInfo(); | |
| for (String sequence : sequences) { | |
| assertTrue( | |
| sequence.matches("^[A-Z](?:/[A-Z][0-9]?(?:[a-z][0-9]?[A-Za-z0-9]*)?)*$"), | |
| "Sequence " + sequence + " should match expected format"); | |
| } | |
| // Check for duplicates | |
| assertEquals( | |
| sequences.size(), | |
| Arrays.stream(sequences.toArray()).distinct().count(), | |
| "Sequences should not contain duplicates"); | |
| } | |
| } | |
| @Test | |
| void testAllEnumValuesHaveSequences() { | |
| // Known MT types that must be present | |
| Set<String> requiredMTs = Set.of("MT101", "MT102", "MT300", "MT508"); | |
| Set<String> actualMTs = Arrays.stream(MtSequenceInfoEnum.values()) | |
| .map(Enum::name) | |
| .collect(Collectors.toSet()); | |
| assertTrue(actualMTs.containsAll(requiredMTs), | |
| "Missing required MT types: " + | |
| requiredMTs.stream() | |
| .filter(mt -> !actualMTs.contains(mt)) | |
| .collect(Collectors.joining(", "))); | |
| for (MtSequenceInfoEnum sequenceInfo : MtSequenceInfoEnum.values()) { | |
| assertNotNull(sequenceInfo.sequencesInfo(), "Sequence info should not be null"); | |
| assertFalse(sequenceInfo.sequencesInfo().isEmpty(), "Sequence info should not be empty"); | |
| // Validate sequence format: | |
| // ^[A-Z] - Starts with uppercase letter | |
| // (?:/[A-Z][0-9]? - Optional parts starting with / followed by letter and optional number | |
| // (?:[a-z][0-9]? - Optional lowercase letter and number | |
| // [A-Za-z0-9]*)?)*$ - Optional additional alphanumeric characters | |
| Set<String> sequences = sequenceInfo.sequencesInfo(); | |
| for (String sequence : sequences) { | |
| assertTrue( | |
| sequence.matches("^[A-Z](?:/[A-Z][0-9]?(?:[a-z][0-9]?[A-Za-z0-9]*)?)*$"), | |
| "Sequence " + sequence + " should match expected format"); | |
| } | |
| // No need for duplicate check as sequences is already a Set | |
| } | |
| } |
ptorres-prowide
force-pushed
the
CU-86b1uerqp_Generar-un-enum-de-MTs-para-validar-sus-secuencias-y-paths
branch
from
0f675b6
to
fbab5c9
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnum.javais excluded by!**/generated/**
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnumTest.java(1 hunks)
🧰 Additional context used
🪛 Markdownlint
CHANGELOG.md
6-6: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
🔇 Additional comments (2)
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnumTest.java (2)
1-13: LGTM! Well-organized imports.
The imports are clean, necessary, and well-organized.
46-54: LGTM! Comprehensive error case testing.
The test method thoroughly covers various invalid scenarios including null, empty, malformed, and non-existent MT numbers.
| // Check for duplicates | ||
| Set<String> uniqueSequences = new HashSet<>(sequences); | ||
| assertEquals( | ||
| sequences.size(), | ||
| uniqueSequences.size(), | ||
| String.format("Sequences in %s contain duplicates: %s", enumName, sequences)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant duplicate check.
The duplicate check is unnecessary since sequences is already a Set (from sequenceInfo.sequencesInfo()). Sets by definition cannot contain duplicates.
- // Check for duplicates
- Set<String> uniqueSequences = new HashSet<>(sequences);
- assertEquals(
- sequences.size(),
- uniqueSequences.size(),
- String.format("Sequences in %s contain duplicates: %s", enumName, sequences));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check for duplicates | |
| Set<String> uniqueSequences = new HashSet<>(sequences); | |
| assertEquals( | |
| sequences.size(), | |
| uniqueSequences.size(), | |
| String.format("Sequences in %s contain duplicates: %s", enumName, sequences)); | |
| assertEquals( | |
| sequences.size(), | |
| uniqueSequences.size(), | |
| String.format("Sequences in %s contain duplicates: %s", enumName, sequences)); |
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnumTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnumTest.java
Outdated
Show resolved
Hide resolved
| @CsvSource({ | ||
| "MT101, 'A,B'", | ||
| "MT102, 'A,B,C'", | ||
| "MT300, 'A,B,B/B1,B/B2,C,D,D/D1,E,E/E1,E/E1/E1a,E/E1/E1a/E1a1,F'", | ||
| "MT508, 'A,A/A1,B,B/B1,C'" | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add test cases for edge cases and special sequences.
Consider adding test cases for:
- Empty sequences
- Single sequence messages
- Maximum depth nested sequences
- Messages with special sequence patterns
@CsvSource({
"MT101, 'A,B'",
"MT102, 'A,B,C'",
"MT300, 'A,B,B/B1,B/B2,C,D,D/D1,E,E/E1,E/E1/E1a,E/E1/E1a/E1a1,F'",
- "MT508, 'A,A/A1,B,B/B1,C'"
+ "MT508, 'A,A/A1,B,B/B1,C'",
+ "MT199, ''", // Empty sequence case
+ "MT202, 'A'", // Single sequence case
+ "MT320, 'A/A1/A1a/A1a1/A1a1a'" // Max depth nested sequence
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @CsvSource({ | |
| "MT101, 'A,B'", | |
| "MT102, 'A,B,C'", | |
| "MT300, 'A,B,B/B1,B/B2,C,D,D/D1,E,E/E1,E/E1/E1a,E/E1/E1a/E1a1,F'", | |
| "MT508, 'A,A/A1,B,B/B1,C'" | |
| }) | |
| @CsvSource({ | |
| "MT101, 'A,B'", | |
| "MT102, 'A,B,C'", | |
| "MT300, 'A,B,B/B1,B/B2,C,D,D/D1,E,E/E1,E/E1/E1a,E/E1/E1a/E1a1,F'", | |
| "MT508, 'A,A/A1,B,B/B1,C'", | |
| "MT199, ''", // Empty sequence case | |
| "MT202, 'A'", // Single sequence case | |
| "MT320, 'A/A1/A1a/A1a1/A1a1a'" // Max depth nested sequence | |
| }) |
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnum.java
Outdated
Show resolved
Hide resolved
src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnum.java
Outdated
Show resolved
Hide resolved
src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnum.java
Outdated
Show resolved
Hide resolved
src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnum.java
Outdated
Show resolved
Hide resolved
| @CsvSource({ | ||
| "MT101, 'A,B'", | ||
| "MT102, 'A,B,C'", | ||
| "MT300, 'A,B,B/B1,B/B2,C,D,D/D1,E,E/E1,E/E1/E1a,E/E1/E1a/E1a1,F'", | ||
| "MT508, 'A,A/A1,B,B/B1,C'" | ||
| }) |
| // Check for duplicates | ||
| Set<String> uniqueSequences = new HashSet<>(sequences); | ||
| assertEquals( | ||
| sequences.size(), | ||
| uniqueSequences.size(), | ||
| String.format("Sequences in %s contain duplicates: %s", enumName, sequences)); |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnum.javais excluded by!**/generated/**
📒 Files selected for processing (1)
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceInfoEnumTest.java(1 hunks)
| private Set<String> toSet(String input) { | ||
|
|
||
| if (input == null || input.trim().isEmpty()) { | ||
| return Collections.emptySet(); | ||
| } | ||
| return Arrays.stream(input.split(",")) | ||
| .map(String::trim) | ||
| .filter(s -> !s.isEmpty()) | ||
| .collect(Collectors.toSet()); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add input validation for malformed sequences.
The toSet method should validate the input format before processing. Consider adding validation for:
- Malformed sequence patterns
- Invalid characters in sequences
- Sequences that don't match the SEQUENCE_PATTERN
private Set<String> toSet(String input) {
if (input == null || input.trim().isEmpty()) {
return Collections.emptySet();
}
+ // Validate input format
+ if (!input.matches("^[A-Z][^,]*(,[A-Z][^,]*)*$")) {
+ throw new IllegalArgumentException("Invalid sequence format: " + input);
+ }
return Arrays.stream(input.split(","))
.map(String::trim)
.filter(s -> !s.isEmpty())
+ .peek(s -> {
+ if (!s.matches(SEQUENCE_PATTERN)) {
+ throw new IllegalArgumentException("Invalid sequence: " + s);
+ }
+ })
.collect(Collectors.toSet());
}Committable suggestion skipped: line range outside the PR's diff.
| @Test | ||
| void testFromFieldNameInvalid() { | ||
| assertThrows(NullPointerException.class, () -> MtSequenceInfoEnum.valueOf(null)); | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("INVALID")); | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("")); | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT")); | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT999999")); | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MTxxx")); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add tests for additional edge cases.
Consider adding tests for:
- Leading/trailing whitespace
- Mixed case variations
- Special Unicode characters
@Test
void testFromFieldNameInvalid() {
assertThrows(NullPointerException.class, () -> MtSequenceInfoEnum.valueOf(null));
assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("INVALID"));
assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf(""));
assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT"));
assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT999999"));
assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MTxxx"));
+ assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf(" MT101 "));
+ assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("mt101"));
+ assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT\u00B9\u00B2\u00B3"));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test | |
| void testFromFieldNameInvalid() { | |
| assertThrows(NullPointerException.class, () -> MtSequenceInfoEnum.valueOf(null)); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("INVALID")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT999999")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MTxxx")); | |
| } | |
| @Test | |
| void testFromFieldNameInvalid() { | |
| assertThrows(NullPointerException.class, () -> MtSequenceInfoEnum.valueOf(null)); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("INVALID")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT999999")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MTxxx")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf(" MT101 ")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("mt101")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceInfoEnum.valueOf("MT\u00B9\u00B2\u00B3")); | |
| } |
| @Test | ||
| void testAllEnumValuesHaveSequences() { | ||
| for (MtSequenceInfoEnum sequenceInfo : MtSequenceInfoEnum.values()) { | ||
| String enumName = sequenceInfo.name(); | ||
| assertNotNull( | ||
| sequenceInfo.sequencesInfo(), | ||
| String.format("Sequence info for enum value '%s' is null", enumName)); | ||
| assertFalse( | ||
| sequenceInfo.sequencesInfo().isEmpty(), | ||
| String.format("Sequence info for enum value '%s' is empty. Expected at least one sequence.", enumName)); | ||
|
|
||
| // Validate sequence format | ||
| Set<String> sequences = sequenceInfo.sequencesInfo(); | ||
| for (String sequence : sequences) { | ||
| assertTrue( | ||
| sequence.matches(SEQUENCE_PATTERN), | ||
| String.format( | ||
| "Sequence '%s' in %s should match format: %s", sequence, enumName, SEQUENCE_PATTERN)); | ||
| } | ||
|
|
||
| // Check for duplicates | ||
| Set<String> uniqueSequences = new HashSet<>(sequences); | ||
| assertEquals( | ||
| sequences.size(), | ||
| uniqueSequences.size(), | ||
| String.format("Sequences in %s contain duplicates: %s", enumName, sequences)); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for enum completeness.
The test should verify that all required MT message types are present in the enum.
@Test
void testAllEnumValuesHaveSequences() {
+ // Verify all required MT types are present
+ Set<String> requiredMTs = Set.of(
+ "MT101", "MT102", "MT103", "MT202", "MT300", "MT508", "MT700"
+ );
+ Set<String> actualMTs = Arrays.stream(MtSequenceInfoEnum.values())
+ .map(Enum::name)
+ .collect(Collectors.toSet());
+
+ requiredMTs.forEach(mt ->
+ assertTrue(actualMTs.contains(mt),
+ "Required MT type " + mt + " is missing from enum"));
+
for (MtSequenceInfoEnum sequenceInfo : MtSequenceInfoEnum.values()) {
// ... existing validation ...
}
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (2)
src/generated/java/com/prowidesoftware/swift/model/field/FieldEnum.javais excluded by!**/generated/**src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceEnum.javais excluded by!**/generated/**
📒 Files selected for processing (2)
CHANGELOG.md(1 hunks)src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceEnumTest.java(1 hunks)
🧰 Additional context used
🪛 Markdownlint
CHANGELOG.md
3-3: Expected: h2; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
4-4: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
3-3: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
4-4: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
🔇 Additional comments (2)
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceEnumTest.java (1)
1-100: Verify coverage of all MT types in the codebase.
While the test coverage for the implemented MT types is thorough, we should verify that all MT types used in the codebase are covered by the enum.
CHANGELOG.md (1)
3-5: 🧹 Nitpick (assertive)
The changelog entry should be more descriptive.
The entry for MtSequenceEnum could be expanded to provide more context about its purpose and capabilities.
Apply this diff to enhance the changelog entry:
-#### 9.4.19 - SNAPSHOT
- * Added new `MtSequenceEnum` with all the available inner sequences of specific MT schemas
-
+#### 9.4.19 - SNAPSHOT
+ * Added new `MtSequenceEnum` to validate and manage MT message sequences:
+ - Comprehensive mapping of inner sequences for all MT message types
+ - Support for sequence validation and path resolution
+ - Structured representation of hierarchical sequences (e.g., "A", "A1", "B", etc.)
+ - Enables programmatic sequence validation and navigation
+🧰 Tools
🪛 Markdownlint
3-3: Expected: h2; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
4-4: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
3-3: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
4-4: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceEnumTest.java
Show resolved
Hide resolved
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceEnumTest.java
Show resolved
Hide resolved
Co-Authored-By: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
ptorres-prowide
force-pushed
the
CU-86b1uerqp_Generar-un-enum-de-MTs-para-validar-sus-secuencias-y-paths
branch
from
9f378a8
to
9dadd05
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (2)
src/generated/java/com/prowidesoftware/swift/model/field/FieldEnum.javais excluded by!**/generated/**src/generated/java/com/prowidesoftware/swift/model/mt/MtSequenceEnum.javais excluded by!**/generated/**
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)src/test/java/com/prowidesoftware/swift/model/field/FieldEnumTest.java(1 hunks)src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceEnumTest.java(1 hunks)
🧰 Additional context used
🪛 Markdownlint
CHANGELOG.md
3-3: Expected: h2; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
4-4: Expected: 0; Actual: 2
Unordered list indentation
(MD007, ul-indent)
3-3: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines
(MD022, blanks-around-headings)
4-4: null
Lists should be surrounded by blank lines
(MD032, blanks-around-lists)
🔇 Additional comments (1)
src/test/java/com/prowidesoftware/swift/model/field/FieldEnumTest.java (1)
Line range hint 1-90: Excellent test coverage maintained!
The test suite demonstrates comprehensive coverage with:
- Clear positive test cases for valid inputs
- Thorough negative test cases for invalid inputs
- Good edge case handling (null, spaces, case sensitivity)
- Complete validation across all enum values
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceEnumTest.java
Show resolved
Hide resolved
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceEnumTest.java
Show resolved
Hide resolved
src/test/java/com/prowidesoftware/swift/model/mt/MtSequenceEnumTest.java
Show resolved
Hide resolved
| @Test | ||
| void testFromFieldNameInvalid() { | ||
| assertThrows(NullPointerException.class, () -> MtSequenceEnum.valueOf(null)); | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("INVALID")); | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("")); | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT")); | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT999999")); | ||
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MTxxx")); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add boundary test cases for field name validation.
Consider adding test cases for boundary conditions:
- MT numbers at the edges of valid ranges
- Special characters in different positions
- Maximum length MT numbers
@Test
void testFromFieldNameInvalid() {
assertThrows(NullPointerException.class, () -> MtSequenceEnum.valueOf(null));
assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("INVALID"));
assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf(""));
assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT"));
assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT999999"));
assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MTxxx"));
+ // Boundary conditions
+ assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT0"));
+ assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT1000"));
+ assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT1.2"));
+ assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT_101"));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test | |
| void testFromFieldNameInvalid() { | |
| assertThrows(NullPointerException.class, () -> MtSequenceEnum.valueOf(null)); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("INVALID")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT999999")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MTxxx")); | |
| } | |
| @Test | |
| void testFromFieldNameInvalid() { | |
| assertThrows(NullPointerException.class, () -> MtSequenceEnum.valueOf(null)); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("INVALID")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT999999")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MTxxx")); | |
| // Boundary conditions | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT0")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT1000")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT1.2")); | |
| assertThrows(IllegalArgumentException.class, () -> MtSequenceEnum.valueOf("MT_101")); | |
| } |
…mTest.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…mTest.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
zubri
deleted the
CU-86b1uerqp_Generar-un-enum-de-MTs-para-validar-sus-secuencias-y-paths
branch
zubri
changed the title
Summary by CodeRabbit
MtSequenceEnumto enumerate inner sequences of specific MT schemas.MtSequenceEnum, validating functionality and handling of valid/invalid inputs.FieldEnumTestby replacing parameterized tests with individual test methods for improved clarity and structure.