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 Tests to DoiCleanup #8124

Merged
merged 12 commits into from
Oct 7, 2021
Merged

Add Tests to DoiCleanup #8124

merged 12 commits into from
Oct 7, 2021

Conversation

brunocmo
Copy link
Contributor

@brunocmo brunocmo commented Oct 5, 2021

Adding unit tests to DoiCleanup class, to improve the coverage of the project.

I added 7 test to the file:

  1. cleanupDoientryJustdoi
  2. cleanupDoiEntryJustDoiAllEntries
  3. cleanupDoiEntryDoiFieldsWithoutHttp
  4. cleanupDoiEntryDoiWithSpaces
  5. cleanupDoiJustUrl
  6. cleanupDoiJustNote
  7. cleanupDoiJustEe
  • 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, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Copy link
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.

Thank you for working on tests. Since this also seems to be a coding exercise, some code feedbacks:

Besides small comments, I would suggest to convert all tests checking to a ParameterizedTest. See org.jabref.logic.bst.BibtexCaseChangersTest#testChangeCaseAllLowers for an example. Can be written even more simple in your case --> first paramter the expected entry, the second one the BibEntry to cleanup.

@brunocmo
Copy link
Contributor Author

brunocmo commented Oct 7, 2021

Thank you for working on tests. Since this also seems to be a coding exercise, some code feedbacks:

Besides small comments, I would suggest to convert all tests checking to a ParameterizedTest. See org.jabref.logic.bst.BibtexCaseChangersTest#testChangeCaseAllLowers for an example. Can be written even more simple in your case --> first paramter the expected entry, the second one the BibEntry to cleanup.

Thanks for the feedback and tips. I took your sugestion and changed to ParameterizedTest. Still I don't know if the implementation is correctly, if you can spot anything wrong again I would be thankfully.

@brunocmo brunocmo requested a review from koppor October 7, 2021 03:26
@koppor koppor merged commit dd5faee into JabRef:main Oct 7, 2021
Siedlerchr added a commit that referenced this pull request Oct 10, 2021
* upstream/main: (149 commits)
  Add Tutorials for javafx
  lint
  Add changelot
  Open folder on mac and highlight file
  Add Tests to DoiCleanup (#8124)
  Improve Drag and Drop in Custom Entry types dialog (#8121)
  Show preview also for available styles (#8110)
  udpate to javafx 17.0.0.1
  Don't throw exception when validating invalid paths (#8112)
  Bump jackson-dataformat-yaml from 2.12.5 to 2.13.0
  Bump jackson-datatype-jsr310 from 2.12.5 to 2.13.0
  Bump byte-buddy-parent from 1.11.15 to 1.11.18
  Bump classgraph from 4.8.116 to 4.8.121
  Bump checkstyle from 9.0 to 9.0.1
  remove iso charset, website returns utf8 for icar comp sci only returns one result
  fix springer fetcher Fix computer science fetcher
  Squashed 'buildres/csl/csl-locales/' changes from 7a507fc008..495f888637
  Squashed 'buildres/csl/csl-styles/' changes from 5facb37..3b00357
  Update CHANGELOG.md
  snap: Use lzo compression & switch to core20 base
  ...
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.

2 participants