clearmatics / libsnark

C++ library for zkSNARKs (forked from scipr-lab/libsnark)
https://clearmatics.github.io/libsnark/
Other
0 stars 2 forks source link

Name test files consistently #34

Open AntoineRondelet opened 3 years ago

AntoineRondelet commented 3 years ago

Most of the test files are named test_<filename>.cpp, nevertheless https://github.com/clearmatics/libsnark/blob/develop/libsnark/gadgetlib1/tests/gadgetlib1_test.cpp is named <filename>_test.cpp which contrasts with the rest. This is inconsistent, and this inconsistency is ever more striking on https://github.com/clearmatics/libsnark/pull/29 where a list of test files are added in the gadgetlib1/tests folder and named with a test_ prefix.

We need to bring consistency in how we name test files to have clean includes and a systematic way of creating tests. While the overwhelming set of test files is using the test_ prefix in libff and libsnark, the _test suffix is used in:

As such, I am in favor to adopt the <filename>_test.cpp naming convention for all test files across all projects to keep naming consistent and systematic and have similar looking include directives.

dtebbs commented 3 years ago

At the same time it would be good to name the resulting executables consistently (e.g. if the test source is test_something.cpp, the resulting executable should be test_something. Inconsistencies can make it awkward to invoke a single test.

dtebbs commented 3 years ago

As such, I am in favor to adopt the _test.cpp naming convention for all test files across all projects to keep naming consistent and systematic and have similar looking include directives.

Could you explain the relationship with include directives specifically? I seem to be missing the point.

AntoineRondelet commented 3 years ago

As such, I am in favor to adopt the _test.cpp naming convention for all test files across all projects to keep naming consistent and systematic and have similar looking include directives.

Could you explain the relationship with include directives specifically? I seem to be missing the point.

This is a general point. If files aren't named consistently, then including them will also be inconsistent, e.g.

#include <mylib/test_mytest.tcc>
#include <mylib/mysecondtest_test.tcc>

This is completely inconsistent. We use suffix conventions for some test files that aren't directly associated with a corresponding executable, here are examples:

These are then included, e.g. https://github.com/clearmatics/zeth/blob/master/libzeth/tests/snarks/groth16/groth16_api_handler_test.cpp#L7

While we use the suffix _tests here, the point raised in this issue remains the same: we should use a consistent approach for naming files so that things become systematic and so that the inconsistencies don't contaminate other source files by adding inconsistency in the #include directives.