Skip to content

Rearrange argument order in CitationKeyGeneratorTest and convert some MethodSource to CsvSources#14600

Closed
hisunll wants to merge 7 commits intoJabRef:mainfrom
Zeglow:jabref-14133
Closed

Rearrange argument order in CitationKeyGeneratorTest and convert some MethodSource to CsvSources#14600
hisunll wants to merge 7 commits intoJabRef:mainfrom
Zeglow:jabref-14133

Conversation

@hisunll
Copy link
Copy Markdown

@hisunll hisunll commented Dec 13, 2025

Refactored JabRef#676

We Refactored CitationKeyGeneratorTest to use csv source instead of method source if possible according to test.md.
We are very sorry for not replying quickly enough for the last PR #14133.
Please confirm you want us to do change expected to the first argument because the previous maintainer writes it that way.

Steps to test

  1. Run the test suite:
   ./gradlew test --tests CitationKeyGeneratorTest
  1. Verify all tests pass
  2. Review the refactored code to ensure:
    • All original test cases are preserved
    • Parameterized tests are properly structured
    • Method sources correctly provide test data
  3. Optionally, compare the test coverage before and after to confirm no regression

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • I manually tested my changes in running JabRef (always required)
  • I added JUnit tests for changes (if applicable)
  • [/] I added screenshots in the PR description (if change is visible to the user)
  • [/] I described the change in CHANGELOG.md in a way that is understandable for the average user (if change is visible to the user)
  • [/] I checked the user documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request updating file(s) in https://github.com/JabRef/user-documentation/tree/main/en.

@github-actions github-actions Bot added the status: changes-required Pull requests that are not yet complete label Dec 13, 2025
@calixtus calixtus added dev: code-quality Issues related to code or architecture decisions dev: testing Related to tests labels Dec 14, 2025
@calixtus calixtus changed the title Jabref 14133 Rearrange argument order in CitationKeyGeneratorTest and convert some MethodSource to CsvSources Dec 14, 2025
@Siedlerchr Siedlerchr closed this Dec 21, 2025
@Siedlerchr Siedlerchr reopened this Dec 21, 2025
@github-actions github-actions Bot removed the status: changes-required Pull requests that are not yet complete label Dec 21, 2025
@Siedlerchr Siedlerchr marked this pull request as ready for review December 21, 2025 21:56
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Dec 21, 2025
Copy link
Copy Markdown
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Not sure if you followed other PRs...

Comment on lines -790 to -794
Arguments.of("[keyword1]", "w1"),
// check keywords with space
Arguments.of("[keyword2]", "w2aw2b"),
// check out of range
Arguments.of("[keyword4]", "")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Keep the comments

Comment on lines 819 to 796
return Stream.of(
// all keywords
Arguments.of("[keywords]", "w1w2aw2bw3"),
// check keywords with space
Arguments.of("[keywords2]", "w1w2aw2b"),
// check out of range
Arguments.of("[keywords55]", "w1w2aw2bw3")
Arguments.of("w1w2aw2bw3", "[keywords]"),
Arguments.of("w1w2aw2b", "[keywords2]"),
Arguments.of("w1w2aw2bw3", "[keywords55]")
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Keey the comments

@github-actions github-actions Bot added status: changes-required Pull requests that are not yet complete and removed status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Dec 23, 2025
@koppor
Copy link
Copy Markdown
Member

koppor commented Dec 27, 2025

@hisunll Will you continue here or should we free this for another contributor wanting to learn about high-quality software engineering?

@hisunll
Copy link
Copy Markdown
Author

hisunll commented Dec 28, 2025

@hisunll Will you continue here or should we free this for another contributor wanting to learn about high-quality software engineering?

Thanks for checking in. I won’t be able to continue this in the near term, so please free it up for another contributor.

@koppor koppor closed this Dec 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev: code-quality Issues related to code or architecture decisions dev: testing Related to tests status: changes-required Pull requests that are not yet complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants