Skip to content
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

Added a test case in CitationKeyGeneratorTest.java file #12194

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

yoyounik
Copy link

@yoyounik yoyounik commented Nov 16, 2024

Add test case for LaTeX command stripping in filename generation

Description
This PR adds a new test case to verify that LaTeX commands are correctly stripped from fields used for filename generation. Specifically, it checks if the generated filename removes commands like \mkbibquote while retaining the meaningful text.

The test case I added (testLatexCommandsAreStrippedFromCitationKey) verifies that LaTeX commands are stripped from the citation key when generating filenames. This aligns with the expected behavior described in the issue.

I could not run the test cases due to a dependency issue in my local setup.

Request: Please verify and run the test case to ensure it behaves as intended.
If the current solution does not fully address the issue, feel free to make necessary corrections.

NOTE -
This PR addresses this issue: #12188 (comment)

  • I own the copyright of the code submitted and I licence it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked 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 to the documentation repository.

@yoyounik
Copy link
Author

hey @koppor
I could not run the test cases due to a dependency issue in my local setup.

Request: Please verify and run the test case to ensure it behaves as intended.
If the current solution does not fully address the issue, feel free to make necessary corrections and tell me if any changes i could make.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use OpenRewrite to ensure "modern" Java coding practices.
The issues found can be automatically fixed.
Please execute the gradle task rewriteRun, check the results, commit, and push.

You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "OpenRewrite".

@@ -18,6 +18,7 @@
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.Answers;

import static org.jabref.gui.search.TextFlowEqualityHelper.assertEquals;
Copy link
Member

Choose a reason for hiding this comment

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

Wrong import

@koppor
Copy link
Member

koppor commented Nov 16, 2024

@yoyounik Please reference the issue this PR addresses.

void testLatexCommandsAreStrippedFromCitationKey() throws ParseException {
BibEntry entry = new BibEntry()
.withField(StandardField.TITLE, "Building \\mkbibquote{Community}");
String pattern = "[title]";
Copy link
Member

Choose a reason for hiding this comment

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

Did you read the issue? Did your LLM fail? Which tool.did you use?

[bibtexkey] - [fulltitle]

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I read the issue and understood the problem about LaTeX commands not being stripped correctly from fields used in filename generation.
I used an AI assistant to refine my understanding of the issue and help structure the PR. However, the final implementation and test case were written based on my own understanding. If there is any discrepancy, I take full responsibility for the work.
I used ChatGPT as a supporting tool to draft and clarify ideas.

Copy link
Member

Choose a reason for hiding this comment

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

I quoted the most important part of the issue above. Please adapt the test case accordingly.

Copy link
Author

Choose a reason for hiding this comment

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

Hey @koppor
i guess i need to read the documentation and little more about this to work on this currently.

Copy link
Member

Choose a reason for hiding this comment

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

Hey @koppor
i guess i need to read the documentation and little more about this to work on this currently.

You need to change a single line in your test case. Instead of [title] use the pattern reported by the user

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @koppor , have updated the pattern reported by the user by [bibtexkey] - [fulltitle]

Copy link
Author

Choose a reason for hiding this comment

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

But i guess there is more to the issue

Copy link
Author

Choose a reason for hiding this comment

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

The import order does not match so i guess i will check it once, and some more unit test issue which i am going throw again

Copy link
Author

Choose a reason for hiding this comment

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

Hey @koppor i have made some changes accordingly, but still the OpenRewrite and UnitTest error is remaining, i will be checking it once again by tomorrow. If you have any hint please provide it , Thanks :)

Copy link
Member

Choose a reason for hiding this comment

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

grafik Click on details to see the exact cause of the failure

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use OpenRewrite to ensure "modern" Java coding practices.
The issues found can be automatically fixed.
Please execute the gradle task rewriteRun, check the results, commit, and push.

You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "OpenRewrite".

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use OpenRewrite to ensure "modern" Java coding practices.
The issues found can be automatically fixed.
Please execute the gradle task rewriteRun, check the results, commit, and push.

You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "OpenRewrite".

@@ -105,7 +105,7 @@ public String generateKey(BibEntry entry) {
String currentKey = entry.getCitationKey().orElse(null);

String newKey = createCitationKeyFromPattern(entry);
newKey = replaceWithRegex(newKey);
newKey = newKey.replaceAll("\\\\[a-zA-Z]+\\{([^}]+)\\}", "$1");
Copy link
Member

Choose a reason for hiding this comment

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

No. Please try to understand the comment #12188 (comment).

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

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.

3 participants