-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 #6207
Comments
I'd like to work on that! |
@dimitra-karadima Sure! Just go ahead. Please open a work-in-progress pull request after your first additions. I have no stron oppinion on which classes need testing and which test classes need restructuring. Just work on it as long as you feel comfortable. |
Add the following changes: -remove multiple assert statements in test cases -split each assert statement in different test methods with meaningful test names
* upstream/master: (50 commits) Keep group pane size when resizing window (#6180) (#6423) Changelog: Fix missing citation for biblatex-mla Update AUTHORS Check duplicate DOI (#6333) Fix missing citation for biblatex-mla Change EasyBind dependency (#6480) Add testing of latest dev version as mandatory Squashed 'src/main/resources/csl-styles/' changes from 5dad23d..586e0b8 Fix libre office connection and other progress dialogs (#6478) Fix clear year and month field when converting to biblatex (#6434) Add truncate as a BibTex key modifier (#6427) Add new authors (not all - they need more work) Remove empty line Add simple Unit Tests for #6207 (#6240) Enforce LeftCurly rule (#6452) Implement task progress indicator (and dialog) in the toolbar (#6443) Consider empty brackets Changelog update Added a test Fixed brackets in regular expressions ...
Hello koppor |
Dear Lars and Robert, We aim for 100% code coverage. You can check existing coverage with IntellIJ or Eclipse. You should see which classes have not that many test Update Do not force the 100% coverage; think of good tests. For instance, test hard things such as unlinked files (see JabRef#484 ) instead of tostring. See also https://ondrej-kvasnovsky.medium.com/is-100-test-coverage-worth-it-2e086ca4d41c and https://blog.ndepend.com/aim-100-percent-test-coverage/ I would love to see automated tests for features described in the user documentation. However, this is much work. I assume you checked the integrity package (mentioned in the issue description) for current test coverage? Cheers, Oliver |
Hi Oliver We are a group of four MSc students from the University of Zurich, @ningxie1991, @BShaq, @Davfon, and myself @nasdas-dev. We created a new fork (ningxie1991/jabref) and work on our individual branches there. We will then create a pull request after each segment of work we have completed and reference this issue ticket. Let us know if that works for you! |
For anyone else wanting to contribute some Unit Tests. The TemplateExporter currently have not yet any unit tests, that would also be a great improvement |
Testing should aim for discovering bugs and ease maintenance of code. Thus, test real functionality. Do not test simple getters or toString methods! Tests should focus on ensuring that code change keep desired customer functionality. |
Given the amazing progress over the last few days (especially once all of the above PRs are merged), should we close this issue and return to our "only refactor if you work on this part of the code anyway" policy? (Then I would also suggest to copy most of the issue text to the contribution guide / how to write tests). |
JabRef offers unit testing based on JUnit5. There are still untested functionalities and not good tests. Here are some tasks in that field.
Please first read our writings on testing.
MinifyNameListFormatterTest
,DOICheckTest
, ... in one pull request, reviewing is hard, because they are all unrelated classes.Unit test creation
Creating unit tests can be trained creating tests for classes residing in following packages
org.jabref.logic.integrity
(to be continued)
There is no need to test all classes. Just pick one or two. Testing takes time and one carefully needs to think about the intention of the class to be able to check user-centric good cases and bad case.
Please also think of adding documentation to https://github.com/JabRef/user-documentation after you understood the intension of the class.
Unit tests improvement
split multiple asserts into different test cases.
It is an antipattern having multiple assert statements in a test case. Thus, they should be split - with meantingful test names
Example:
Add paramaterized tests
org.jabref.logic.integrity.IntegrityCheckTest
: split tests into tests for each integrity check class (e.g. checking editions should go toEditionChecker
)(to be continued)
Other tests to work on
The text was updated successfully, but these errors were encountered: