HW-2 Issue676#1
Conversation
- ProtectTermsFormatterTest: Combined individual @test methods into parameterized test - UnprotectTermsFormatterTest: Converted to @ParameterizedTest with comprehensive test cases - AbbreviationParserTest: Combined delimiter tests into single parameterized test - AbbreviationTest: Converted 13 test methods to parameterized approach - AbbreviationWriterTest: Combined writing tests into parameterized test - AbbreviationsTest: Converted abbreviation lookup tests to parameterized format - JournalInformationFetcherTest: Organized 12 tests into logical parameterized groups All conversions maintain original test functionality while following JUnit 5 best practices
|
I can't review this because it includes other changes. We can discuss this at our appointment. |
|
Could you please come in person to my Wednesday office hours this week? |
espertusnu
left a comment
There was a problem hiding this comment.
It looks like some of my previous comments might not have gone through. Sorry about that!
| } | ||
| parser.readJournalListFromFile(csvFile); | ||
| assertEquals(Set.of(abbreviation), parser.getAbbreviations()); | ||
| private static Stream<Arguments> provideDelimiterTestCases() { |
There was a problem hiding this comment.
I think these argument providers usually go right above the test that uses them, but correct me if I'm wrong.
There was a problem hiding this comment.
Please respond to this comment.
There was a problem hiding this comment.
Yes you are right, positioning a @MethodSource providers directly above the tests improves readability
| assertEquals("L. N.", abbreviation.getShortestUniqueAbbreviation()); | ||
| @ParameterizedTest(name = "{0}") | ||
| @MethodSource("provideAbbreviationTestCases") | ||
| void testAbbreviationProperties(String testName, Abbreviation abbreviation, |
There was a problem hiding this comment.
Try starting a new line after the left parenthesis.
There was a problem hiding this comment.
Apologies for the oversight, I’ll reformat the method signature to break after the left parenthesis and wrap the parameters consistently.
espertusnu
left a comment
There was a problem hiding this comment.
@sandeepsamueljayakumar I re-reviewed your code as you requested, but you still had not responded to all my comments. Please view the PR conversation on github and scroll down the page to see comments that have not been responded to. I will wait until you have responded to all outstanding comments before doing further review of your code.
Please ask if you don't understand how to view comments. That is also something that Hazel could help you with.
Homework 2 - Issue 676 Pull Request
As a team, we discussed and I took logic.casechanger and logic.journals
And I made sure,
-There are no Checkstyle issues.
-There are no SpotBugs issues.
-There are no javadoc errors.
-All tests in the file pass.
Will you get full credit on this assignment?
-I'm hoping to get full credit on thie assignment.
How long did you spend on the assignment?
-I spent around 4-5 hours totally.
What problems did you encounter, and how did you deal with them?
-I had some issues with installing jabref, where i got some errors like mising JAVA_HOME, then I used chatgpt to learn what it was about and what i could do to fix it. then it suggested to set JAVA_HOME as environment variable, which I did, then everything worked out perfectly.
Fixes JabRef#676
What help did you give or receive on this assignment (including answering these question)? What was your AI usage?
-I used Chatpgt 5 to check if the changes i made (parameterized tests) were correct, and used AI during installing jabref.