Task #651
Increase coverage of selection unit tests
Description
- Each selection keyword should work in isolation.
- Simple and complex boolean and arithmetic operations, both with dynamic and static values.
- Use of variable assignment in different contexts, both with the variable used once or more than once (the latter is more important).
- Complex boolean operations that involve shared subexpressions (through variables) that require both static and dynamic evaluation.
- Failure recovery from parse, compile, and evaluation errors.
- Coverage of all cases of special evaluation methods used for comparison and position evaluation.
Separate unit tests for index group operations (indexutil.h), position calculation (poscalc.h) and neiighborhood searching (nbsearch.h) would also be useful, although there isn't any pressing need to change these parts in the near future.
Related issues
Associated revisions
Added test framework for reference runs.
The framework allows one to write a test such that it can be invoked to
produce its own reference data that subsequent runs are compared
against.
Functionality in preparation of IssueID #651.
Added selection unit tests using new framework.
Added unit tests (or converted old ones) for simple selection keywords
using the new reference data framework. This didn't yet increase the
coverage significantly, but new tests should now be easy to add: one
just has to add a test with the selections to be tested (and possibly
prepare input topology/coordinate files, at least a PDB file would be
needed for some keywords), run that test to create the reference data,
and check that the reference data is correct.
IssueID #651
Converted rest of selection unit tests.
Now all existing unit tests use the new reference data framework.
Added support for using variables in the selections to be tested.
IssueID #651
Added selection unit tests for most keywords.
Now all single keywords are either tested or mentioned in a todo comment
in the test source file. The ones that are not would mostly require
different kinds of input files (pdb and/or tpr) with all kinds of fancy
features.
One of the tests fails, but will fix the bug in a separate commit.
IssueID #651
Add tests for PDB-specific selection methods.
Part of issue #651.
Change-Id: I6d2d7489ded6c6a87ffda5a86d9ef6e5a9e6112f
Increase coverate of selection unit tests.
Add some tests and adjust existing tests to cover parts of the selection
compiler and evaluation functions that the coverage run showed as not
covered.
Fixed one issue found (can be backported to 4.5 if necessary).
Part of #651.
Change-Id: If92b5b1eb8cf6b3acb510ba43f64fc82b63ab82a
More thorough unit tests for plus/merge selections.
- Test also the return value of mappedId() and refId() from selected
positions.
- Adjust tested selections to better cover non-atom positions.
- Add tests for dynamic selections with plus/merge.
Part of #651.
Change-Id: I4f9266fd9e1b02d414ed403a182124b244b253db
More selection unit tests for variables and fixes.
- Added unit tests for more complex selections where either numeric or
position values were assigned to variables.
- Fixed handling of subexpressions in these cases (will back-port to
older branches separately).
- Added unit tests for constant expressions in variables (in particular
constant positions were not handled right during initial development
of selections).
- Added comments in the code for some parts that are currently
unreachable.
Part of #651.
Change-Id: Ifbb585ca00fd7a9715b4c9cb004c3a895551c1a6
Unit tests for insolidangle selections.
The current input data is not particularly elaborate, and more careful
selection could possibly test more corner cases. But coverage even with
the current 500+500 random test points should be relatively good.
In addition to the tests themselves and input data for them,
added a Python script that was used to generate the test data.
Part of #651.
Change-Id: I80acd431b2963bf65fbeed53fe6be4f885d07ec5
Improve reference data checks against same data.
Make it easier to perform multiple checks against the same reference
data using the TestReferenceChecker. nextSearchNode_ was always NULL
when the reference data was getting written, which caused every check to
write a new reference data entry. Now, the search is also performed as
part of writing the reference data, so that only the first check with a
particular id creates an entry, and subsequent checks perform the check
against this entry.
The analysis data tests were depending on duplicate ids behaving
differently, so changed them to also use the same pattern as elsewhere
for repeating elements, i.e., NULL ids.
Related to #651.
Change-Id: Icc0d1d68a0846bbdcc9639933e8060e56cb4b30e
Separate topology handling in selection tests.
Split common functionality for handling test topologies in selection
unit tests into a separate files (toputils.*). This commit does not yet
add a lot of reusable code, but subsequent commits will (and they will
also introduce more files that use this functionality).
Part of #651.
Change-Id: Idda0099ba166014c9d6c36e0d8ba212e5fa8e940
Add unit tests for selection index mapping.
Add unit tests for parts of indexutil.* that are mainly used by the
selection position calculation engine.
Add required functionality to toputils.* and update typedefs.c to
support more flexible freeing of required data structures.
Part of #651, related to #1221.
Change-Id: Ibc68ad71a4834b991820014969be46152426a9f5
Proper C++ API for analysis neighborhood search.
Replace the dummy C++ wrapper with a more C++-like API for the
neighborhood search routines that are part of the selection code.
Subsequent commits will implement a few additional features to the API,
but split those off from here to keep things easier to review.
Implementation underneath is still the same as earlier.
Will remove the old C functions in a separate commit.
Changed the free volume tool to use the C++ API in preparation for this.
Added some basic tests for the functionality using the new API, which
should increase the percentual coverage in this part of the code
significantly.
Change-Id: I139866b82cb048f1676d187b0cf6e5d270d4d32b
Add unit tests for selection position calculation.
Add unit tests for most functionality of poscalc.*. These tests also
cover centerofmass.*.
Fixed two issues:
- POS_MASKONLY calculations had an inconsistent group set in the output
positions (which was visible in the selection interface, returning
incorrect set of atoms for the selection).
- Force calculation was incorrect.
These do not affect g_select, so only fixed for 5.0.
Part of #651, related to #1221.
Change-Id: I4dc6475f53fb3b1559bae9296f8c9f3e6dd14bf7
Add unit test coverage for simple selection keywords.
"molindex" and "atomtype" keywords are now covered by separate tests.
Also updated one test to use the "z" keyword to have that covered.
Added a comment in sm_simple.cpp about unreachable code that is not
currently covered by tests.
Part of #651.
Change-Id: Id9ac9ea8e7d144214537265d9ed89335684934f1
Add unit tests for index groups in selections.
Add tests for using the "group" keyword in selections to select external
index groups, and in particular for directly providing an index group as
the selection value.
Fixed a master-only issue in handling the selection name in the latter
cases. Also fixed master-only memory leaks and other issues in
gmx_ana_indexgrps_init().
Part of #651.
Change-Id: I0d908213f109ced9ab6708e55c6acf53371f6aa1
Add check for unsorted index groups in selections.
Selections did not work properly if an index group was used that
did not have the atom indices in strictly ascending order.
Added a check for this, and tests that check that this now gives an
error.
Refactored the way index group references are resolved such that the
logic and the related error messages are now only in one place:
selelem.cpp. This resolves TODOs about improving the error messages.
Also made this piece of code to use exceptions for error reporting.
Change-Id: I2318823b1be74775d0a0f8bd4a3c58b0aed1aba1
Remove superfluous selection names from refdata.
Now that there are proper tests for handling the selection names,
removed the testing of the name from most other tests, since the name
was always identical to the selection text.
Part of #651.
Change-Id: I3844031f94336c9934d79fec711920eed04cea4a
History
#1 Updated by Teemu Murtola almost 10 years ago
- Assignee set to Teemu Murtola
#2 Updated by Teemu Murtola almost 10 years ago
- Status changed from New to In Progress
There's now a generic framework that should make it easy to add new selection tests; as long as new types of input are not needed (external index files or real trajectories are not yet supported), very little programming is required to add a new test. The coverage of the tests still needs improving.
#3 Updated by Teemu Murtola over 9 years ago
- Description updated (diff)
Added a few more areas where automatic tests could be useful.
#4 Updated by Teemu Murtola about 8 years ago
- Target version set to 5.0
There is already a lot more tests, and code coverage of the general selection compilation is very high. Some selection keywords, some special cases, and many error paths are not covered. Keeping the issue open for linking additional commits to it, but setting the target version to 5.0 to avoid keeping it open-ended.
#5 Updated by Teemu Murtola over 7 years ago
- Status changed from In Progress to Fix uploaded
Most normal functionality should now be covered by the tests added in commits leading to https://gerrit.gromacs.org/#/c/2365/. When these get merged together with https://gerrit.gromacs.org/#/c/2335/, these should be sufficient to mark this issue as resolved. A final check on the test coverage is in order, but it should be mostly some error paths and some conditionals that are not covered. From now on, it is probably most productive to add new tests when developing new features or when bugs are found.
#6 Updated by Teemu Murtola about 7 years ago
- Project changed from Next-generation analysis tools to GROMACS
- Category set to selections
- Status changed from Fix uploaded to Resolved
#7 Updated by Rossen Apostolov about 7 years ago
I'm closing this one. Teemu, please reopen if needed.
#8 Updated by Rossen Apostolov about 7 years ago
- Status changed from Resolved to Closed
Added functions for path and dir manipulation.
These functions should hide OS-specific functionality behind a common
interface. Tried to implement them using only POSIX functions, so
hopefully they'll work at least on Linux (tested on OS X). Should add
separate implementations for Windows.
Low-level functionality in preparation for IssueID #651.