Skip to content

Add simple unit tests#7696

Merged
Siedlerchr merged 3 commits into
JabRef:mainfrom
ningxie1991:a4-df3
May 13, 2021
Merged

Add simple unit tests#7696
Siedlerchr merged 3 commits into
JabRef:mainfrom
ningxie1991:a4-df3

Conversation

@Davfon

@Davfon Davfon commented May 3, 2021

Copy link
Copy Markdown
Contributor

This pull request contributes to issue #6207, which is to add more unit tests or improve existing ones.

I added the trivial, but important, test case, that the output should remain unmodified in case the input was already valid.

Tests extended:

  • CleanupUrlFormatterTest
  • RemoveHyphenatedNewlinesFormatterTest
  • RemoveNewlinesFormatterTest
  • LowerCaseFormatterTest
  • RemoveBracketsTest
  • 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 documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

Davfon added 2 commits May 2, 2021 15:00
Add elemental test case for formatters, to check that a valid input remains unmodified.

@Test
void validURLUnmodified() {
// the caller has to pay attention that this does not happen

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.

What should not happen? Maybe just remove the comment.

The call seems IMHO legal.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's on me, I copy-pasted and forgot to remove it.

}

@Test
void validURLUnmodified() {

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.

We try to gradually migrate to Google's casing rules: https://google.github.io/styleguide/javaguide.html#s5.3-camel-case

Thus, please introduce defined camel case

Suggested change
void validURLUnmodified() {
void validUrlUnmodified() {


@Test
public void test() {
assertEquals("lower", formatter.format("lower"));

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.

Could you maybe change that to a paramterized test? We aim for having one assertion per test case. See also #6207

grafik

@koppor koppor added the status: changes-required Pull requests that are not yet complete label May 6, 2021
Address the requested changes from #7696
@Davfon

Davfon commented May 12, 2021

Copy link
Copy Markdown
Contributor Author

I addressed the requested changes in the last commit, should be all good now.

@Siedlerchr Siedlerchr merged commit b9c48f5 into JabRef:main May 13, 2021
Siedlerchr added a commit that referenced this pull request May 15, 2021
* upstream/main: (71 commits)
  [Bot] Update CSL styles (#7735)
  Fix for issue 6966: open all files of multiple entries (#7709)
  Add simple unit tests (#7696)
  Add simple unit tests (#7543)
  Update check-outdated-dependencies.yml
  Added preset for new entry keybindings and reintroduced defaults (#7705)
  Select the entry which has smaller dictonary order when merge (#7708)
  Update CHANGELOG.md
  fix: make more fields, fomatters, ids and languages sorted by alphabetical order (#7717)
  Bump libreoffice from 7.1.2 to 7.1.3 (#7721)
  Bump unoloader from 7.1.2 to 7.1.3 (#7724)
  Bump org.beryx.jlink from 2.23.7 to 2.23.8 (#7723)
  Bump org.openjfx.javafxplugin from 0.0.9 to 0.0.10 (#7725)
  fix: make fields sorted by lexicographical order (#7711)
  Fix tests
  Refactoring existing unit tests (#7687)
  Refactoring and addition of unit tests (#7581)
  Refactor simple Unit Tests (#7571)
  Add simple unit tests (#7544)
  add and extend unit tests (#7685)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: changes-required Pull requests that are not yet complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants