Project

General

Profile

Task #651

Increase coverage of selection unit tests

Added by Teemu Murtola almost 9 years ago. Updated almost 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
selections
Target version:
Difficulty:
uncategorized
Close

Description

As it is now, selection unit tests don't test the actual selection engine very thoroughly. Before doing any larger changes to the selection framework, it would be very good to add tests at least for:
  • 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.
Additional features that would benefit from unit tests:
  • 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

Related to GROMACS - Task #866: Improve analysis neighborhood search APIClosed01/25/2012
Related to GROMACS - Feature #1221: More generic position mapping for selectionsAccepted04/12/2013
Blocks GROMACS - Task #652: Change selection method implementation to use C++Blocked, need info

Associated revisions

Revision 094e216e (diff)
Added by Teemu Murtola over 8 years ago

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.

Revision d775fe19 (diff)
Added by Teemu Murtola over 8 years ago

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.

Revision b76ef18a (diff)
Added by Teemu Murtola over 8 years ago

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

Revision 01c0352b (diff)
Added by Teemu Murtola over 8 years ago

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

Revision 18f8b890 (diff)
Added by Teemu Murtola over 8 years ago

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

Revision d10a77b6 (diff)
Added by Teemu Murtola almost 8 years ago

Added unit tests for some selection errors.

Part of tasks #651 and #655.

Change-Id: I3a6a9bb0e4e635eb93f5c5faa4e796415592b608

Revision 2a394567 (diff)
Added by Teemu Murtola over 7 years ago

Add tests for PDB-specific selection methods.

Part of issue #651.

Change-Id: I6d2d7489ded6c6a87ffda5a86d9ef6e5a9e6112f

Revision dd3a1029 (diff)
Added by Teemu Murtola over 7 years ago

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

Revision fa4da1bd (diff)
Added by Teemu Murtola about 7 years ago

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

Revision 56006661 (diff)
Added by Teemu Murtola about 7 years ago

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

Revision 98b0cb23 (diff)
Added by Teemu Murtola about 7 years ago

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

Revision fc2e366c (diff)
Added by Teemu Murtola over 6 years ago

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

Revision d199b667 (diff)
Added by Teemu Murtola over 6 years ago

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

Revision 2504ba9a (diff)
Added by Teemu Murtola over 6 years ago

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

Revision dabd9a3c (diff)
Added by Teemu Murtola over 6 years ago

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.

Part of #866 and #651.

Change-Id: I139866b82cb048f1676d187b0cf6e5d270d4d32b

Revision 49ec30f1 (diff)
Added by Teemu Murtola over 6 years ago

Fix analysis neighborhood search for 2D/screw PBC.

The gridding implementation did not work correctly in these cases, but
could get triggered. For now, a simple all-pairs search is always used
in these cases.

Related to #866 and #651.

Change-Id: Iad066f3ea5964e97e1a29a0a20c5725f205509c8

Revision b6a7c327 (diff)
Added by Teemu Murtola about 6 years ago

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

Revision decc828a (diff)
Added by Teemu Murtola about 6 years ago

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

Revision 54903092 (diff)
Added by Teemu Murtola about 6 years ago

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

Revision 907e5518 (diff)
Added by Teemu Murtola about 6 years ago

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.

Related to #651 and #655.

Change-Id: I2318823b1be74775d0a0f8bd4a3c58b0aed1aba1

Revision ec3b4b10 (diff)
Added by Teemu Murtola about 6 years ago

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 over 8 years ago

  • Assignee set to Teemu Murtola

#2 Updated by Teemu Murtola over 8 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 about 8 years ago

  • Description updated (diff)

Added a few more areas where automatic tests could be useful.

#4 Updated by Teemu Murtola almost 7 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 6 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 almost 6 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 almost 6 years ago

I'm closing this one. Teemu, please reopen if needed.

#8 Updated by Rossen Apostolov almost 6 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF