Skip to content

Add test for [authorIni]#9799

Merged
Siedlerchr merged 2 commits into
mainfrom
addAuthorInitest
Apr 26, 2023
Merged

Add test for [authorIni]#9799
Siedlerchr merged 2 commits into
mainfrom
addAuthorInitest

Conversation

@koppor

@koppor koppor commented Apr 25, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

No functionality change, just a test to ensure everything works.

Also adds a more "readable" tests for patterns:

    @CsvSource({
            "'Newton', '[auth]', 'Isaac Newton'",
            "'Newton', '[authFirstFull]', 'Isaac Newton'",
            "'I', '[authForeIni]', 'Isaac Newton'",
            "'Newton', '[auth.etal]', 'Isaac Newton'",
            "'Newton', '[authEtAl]', 'Isaac Newton'",
            "'Newton', '[auth.auth.ea]', 'Isaac Newton'",
            "'Newton', '[authors]', 'Isaac Newton'",
            "'Newton', '[authors2]', 'Isaac Newton'",
            "'Ne', '[authIni2]', 'Isaac Newton'",
            "'New', '[auth3]', 'Isaac Newton'",
            "'New', '[auth3_1]', 'Isaac Newton'",
            "'Newton', '[authshort]', 'Isaac Newton'",
            "'New', '[authorsAlpha]', 'Isaac Newton'",
            "'Newton', '[authorLast]', 'Isaac Newton'",
            "'I', '[authorLastForeIni]', 'Isaac Newton'",

            "'Agency', '[authors]', 'European Union Aviation Safety Agency'",
            "'EUASA', '[authors]', '{European Union Aviation Safety Agency}'"
    })
### Compulsory checks
- [ ] Change in `CHANGELOG.md` described in a way that is understandable for the average user (if applicable)
- [ ] 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](https://devdocs.jabref.org/): Is the information available and up to date? If not, I outlined it in this pull request.
- [ ] [Checked documentation](https://docs.jabref.org/): 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.

@koppor koppor added dev: code-quality Issues related to code or architecture decisions status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Apr 25, 2023
@Siedlerchr Siedlerchr merged commit ba479d5 into main Apr 26, 2023
@Siedlerchr Siedlerchr deleted the addAuthorInitest branch April 26, 2023 19:31
@Siedlerchr Siedlerchr mentioned this pull request May 19, 2023
Merged
@vicluo96

vicluo96 commented May 20, 2023

Copy link
Copy Markdown
Contributor

Hi, I would like to add more test cases for this issue, is there still anything I can contribute to the test suite?

@koppor

koppor commented May 21, 2023

Copy link
Copy Markdown
Member Author

@vicluo96 You hit a very special pull request. This pull request does not fix any issue, but adds test cases for authorLni. - You can go through the list at https://docs.jabref.org/setup/citationkeypatterns and check for missing test cases. Probably, you will have to improve the code, too. The "and others" case might have been missed some times. See the diff at the pull request #9703 (click on the "Files" tab).

@vicluo96

vicluo96 commented May 21, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

@koppor I was reviewing on the test cases related to citation key generators.
Screen Shot 2023-05-20 at 8 19 41 PM

I am trying to write a new test cases with a potential edge case input, and I find that most of the cleankey works, but there stills something that may be omitted. For example, when I try to add a author "Giannis Adétòkunbọ̀", the last character isn't replaced.
Screen Shot 2023-05-20 at 8 30 26 PM
Is this a new issue? Could I wirte add some inputs in the existing test case or write a new test case for that?

@koppor

koppor commented May 21, 2023

Copy link
Copy Markdown
Member Author

It seems you found an issue. Yes, please go ahaed and add a test case as well as a fix!

The screenshoted test case is also not of good quality. Should be rewritten to a @ParameterizedTest.

@calixtus

calixtus commented May 21, 2023

Copy link
Copy Markdown
Member

Careful, we just merged a pull request changing those monstrous citationkeypatterntests to parameterized tests.

@calixtus

calixtus commented May 21, 2023

Copy link
Copy Markdown
Member

#9915

Please update your main branch.

@vicluo96 vicluo96 mentioned this pull request May 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

@calixtus calixtus calixtus approved these changes

@k3KAW8Pnf7mkmdSMPHz27 k3KAW8Pnf7mkmdSMPHz27 Awaiting requested review from k3KAW8Pnf7mkmdSMPHz27

Assignees

No one assigned

Labels

dev: code-quality Issues related to code or architecture decisions status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@koppor @vicluo96 @calixtus @Siedlerchr