test/vis.cpp #includes a non-installed header

Building test/vis.cpp using an installed copy of UPC++ on any system gets a compilation error:

vis.cpp:10:26: fatal error: upcxx/wait.hpp: No such file or directory
compilation terminated.

The problem is this header is not currently included by upcxx.hpp and hence also not installed by nobs. This renders this test broken for use with an installed UPC++.

Our CI currently builds tests in the "tests" directory using nobs on the build tree (rather than upcxx-meta from the installation), which is why this defect went undetected for so long.

@bvstraalen : I don't understand why this header was included here in the first place, but deleting this line resolves the problem, so that's probably the right solution in this case.

@jdbachan : why are general tests like this one including individual (undocumented/unspecified) headers, instead of the only documented header <upcxx/upcxx.hpp> that pulls in everything in the spec? I understand the desire to unit test, but the former practice seems error-prone and could leave defects like this one potentially undetected for a long time. Also, if we ever had an error where some spec-required functionality was provided by an undocumented sub-header but not the documented top-level header, we may fail to detect that error entirely.

As a general testing practice, it seems preferable for correctness tests to always use the available specified/documented interfaces, unless there is a very strong reason why they cannot (one example being access to unspecified internals, which does not apply to this test).

Comments (2)