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

Add simple unit tests #7696

Merged
merged 3 commits into from
May 13, 2021
Merged

Add simple unit tests #7696

merged 3 commits into from
May 13, 2021

Conversation

Davfon
Copy link
Contributor

@Davfon Davfon commented May 3, 2021

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.
@@ -29,6 +29,12 @@ void extractURLFormLink() {
formatter.format("away.php?to=http%3A%2F%2Fwikipedia.org&a=snippet"));
}

@Test
void validURLUnmodified() {
// the caller has to pay attention that this does not happen
Copy link
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
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.

@@ -29,6 +29,12 @@ void extractURLFormLink() {
formatter.format("away.php?to=http%3A%2F%2Fwikipedia.org&a=snippet"));
}

@Test
void validURLUnmodified() {
Copy link
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() {

@@ -19,6 +19,7 @@ public void setUp() {

@Test
public void test() {
assertEquals("lower", formatter.format("lower"));
Copy link
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
Copy link
Contributor Author

Davfon commented May 12, 2021

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