Make Test Design More Uniform

[rheitjoh]

In the process of writing tests for group signatures, I have noticed some potential for improvement in the way the tests are written.
Specifically, the "scheme tester" type of general test classes such as StandaloneTest, EncryptionSchemeTest, SignatureSchemeTester, etc.

There is a a clear difference in design between some of these classes. They can be split into two categories:

• The StandaloneTest type of tests. Here the test class is already implemented and the developer only has to provide a set of parameters that are used in the test. Implemented using JUnit's test parameterization features. The StandaloneTest also uses Reflection to automatically collect relevant tests.

• The SignatureSchemeTester type of tests. Here the test class provides methods that can be included in a test case to carry out some standard testing tasks such as running sign and verify. The developer has to write the test class themselves.

This difference in design has some problems in my opinion. The developer needs to familiarise themselves with both variants.
To improve usability of the library, we should therefore decide on a common test design and abide by it for all relevant tests.

I would propose we move all tests toward StandaloneTest type of tests. These have the advantage that they automatically collect any scheme implementations, and, with this, can inform the developer of where to find the respective tests. The error message can also tell the developer how to actually implement the tests correctly and where to find the relevant classes. I have implemented this type of approach in the group signature tests as well.

Furthermore, they reduce the work of the developer compared to something like SignatureSchemeTester since the developer has to only provide a set of fixed parameters, while the former requires creating a whole new test class plus a set of parameters.

If you do not want the tests to run, you can still disable them for your scheme by removing the Class entry from the set of collected classes.

[feidens]

I vote for your suggestion. A unification would be great.

One question: Isn't there a reason why the SignatureSchemeTester exists and is differently designed? Someone decided to use a different test design, maybe the person had a reason for that. Do you have an idea why?

[JanBobolz]

Sounds reasonable.

While we're at it: can we maybe replace the collection of parameters with reflection magic, too? Currently, we have this de-facto standard of classes with a single static method to provide test parameters, but we still have to manually add it to the test parameter list in StandaloneTest.
Could StandaloneTest just use reflection to collect all such classes and then just add their parameters automatically?

I want to mention: if we go this route, test code becomes less useful for newcomers to the library. The SignatureSchemeTester route had the advantage that you can immediately see how to (1) instantiate and (2) use a scheme.
The more reflection magic we put between the code that does something with signatures and the code that instantiates them, the less useful the tests become in their "documentation" role. In other words: the most naive tests (just a simple class with no inheritence or whatever) are actually perfect "minimal examples" of how to use a signature scheme and we're moving away from this more and more.

[rheitjoh]

One question: Isn't there a reason why the SignatureSchemeTester exists and is differently designed? Someone decided to use a different test design, maybe the person had a reason for that. Do you have an idea why?

I am not sure. From looking at the original sources on the university gitlab, I can tell that those tests were written by different persons. I skimmed over the list of issues and did not notice any test design discussions.

Except from Jan's remark about the test code being a form of documentation in itself, I don't think the SignatureSchemeTester approach really offers any more advantages.

Another disadvantage of the SignatureSchemeTester approach is that the developer has to be familiar with JUnit themselves.

I want to mention: if we go this route, test code becomes less useful for newcomers to the library. The SignatureSchemeTester route had the advantage that you can immediately see how to (1) instantiate and (2) use a scheme.
The more reflection magic we put between the code that does something with signatures and the code that instantiates them, the less useful the tests become in their "documentation" role. In other words: the most naive tests (just a simple class with no inheritence or whatever) are actually perfect "minimal examples" of how to use a signature scheme and we're moving away from this more and more.

I would prefer this to be in the actual documentation for the interface itself. You can format code in there just fine.
Plus that is where I would expect such documentation to be, personally.

There is not really much reflection magic necessary aside from what is used to collect the implementing classes. I think that complexity is more due to how StandaloneRepresentable works, specifcally.

If we look at the group signature tests I am writing at the moment, then we can see that they are more complex than signature tests, but I think that has more to do with trying to optimize defect localization and feedback to the developer.
By defect localization I mean that, if e.g. the join protocol fails, not every test fails (which is confusing). Instead, only the join test fails and all the other tests automatically get ignored. This improves defect localization, i.e. being able to identify the issue that produced the error.

Furthermore, the GroupSignatureScheme aims to be compatible with many different group signature schemes. To that end, a high number of interface methods exist that support the different capabilities of various group signatures such as claim proofs and traceability. The tests allow the user to indicate that a method is not implemented by throwing a UnsupportedOperationException. This is nice because it enforces uniformity when it comes to handling such not offered methods. We won't have some schemes throwing different exceptions or just returning null.

[rheitjoh]

While we're at it: can we maybe replace the collection of parameters with reflection magic, too? Currently, we have this de-facto standard of classes with a single static method to provide test parameters, but we still have to manually add it to the test parameter list in StandaloneTest.
Could StandaloneTest just use reflection to collect all such classes and then just add their parameters automatically?

That should not be a problem to add. I think then it would make sense to create another interface for these parameter provider classes since the get() method is actually not required right now, you could use any other way to provide the relevant parameters.

[feidens]

I would prefer this to be in the actual documentation for the interface itself. You can format code in there just fine.
Plus that is where I would expect such documentation to be, personally.

Can you please link us to an example?

[rheitjoh]

I would prefer this to be in the actual documentation for the interface itself. You can format code in there just fine.
Plus that is where I would expect such documentation to be, personally.

Can you please link us to an example?

Its only used in here at the moment. For the precomputePow() methods.

[feidens]

I would prefer this to be in the actual documentation for the interface itself. You can format code in there just fine.
Plus that is where I would expect such documentation to be, personally.

Can you please link us to an example?

Its only used in here at the moment. For the precomputePow() methods.

For me there is a slight difference between what @JanBobolz mentioned (see in the tests how an API is used or instantiated) and what you are suggesting. Your vision is that a developer can see how to use it if she visits our documentation and looks up the interface?

[JanBobolz]

Indeed. Sometimes I just want to know "hey, if I quickly want to encrypt something with ElGamal, how do I do it?". My usual workflow then goes to the ElGamal class, I'll let the IDE look up where the constructor is ever referenced, and voilá, there's my example code in the corresponding test case.

That is a very different (and, depending on use-case, much more accessible) form of documentation than the EncryptionScheme's interface Javadoc, which, for example, doesn't really tell me how to instantiate the ElGamal class, and is much more wordy than just plain test code encrypting and decrypting stuff concretely.

Anyway; I'll let you guys decide what to do with that argument.

[rheitjoh]

doesn't really tell me how to instantiate the ElGamal class

The different test designs make no difference here since they both instantiate stuff in a parameter class.

much more wordy than just plain test code encrypting and decrypting stuff concretely

I don't see why the interface javadoc should be really any more wordy. You can still have a short code snippet at the top which tells the user how it works. No need to read through the whole interface doc. If there are any scheme-specific things that would not be covered in the general interface, they should be in the scheme's own javadoc, and not hidden in the tests somewhere.

For me there is a slight difference between what @JanBobolz mentioned (see in the tests how an API is used or instantiated) and what you are suggesting. Your vision is that a developer can see how to use it if she visits our documentation and looks up the interface?

They don't need to open the documentation page to look at the javadoc, they can just do that in code. You can still do the same thing, let the IDE find the references, and then just go to the interface. That will tell you more than the tester class does currently.

I don't actually find that the "find references in code" is such a good idea for finding out how to use the whole thing, anyways. If you are unlucky, you will get a long list of files that don't really help you with your problem because they are not meant to be used as documentation. The ElGamal encrypt() method, for example, leads me to 20 different files if I press Ctrl+M1 or Ctrl+B on it.

[JanBobolz]

Agreement

After taking a closer look at the signature scheme tests, I'm hard agreeing with Raphael on this. The current way is ... not great. There's way too much copy/paste code in there. For every test method I "generically" use, I still need to write code to call the generic test method with its specific input.

However, going the full StandaloneTest route may be a bit too constrained, because there is no good way to implement custom scheme-dependent tests in that case (which is fine for the StandaloneTests as they're intentionally constrained to a fixed set of "does this class fulfill the contract" tests, but not great for things like SPSEQ with additional functionality).

Proposal

I propose the following solution.

• Set up a hierarchical solution, i.e. we implement a class SignatureSchemeTest with a bunch of (jUnit5) @Test methods and an abstract getParameters() method that is called by the @Tests.
• Implementors of a specific scheme would implement a PointchevalSandersTest extends SignatureSchemeTest, override the getParameters() method (suggestion: set up scheme and parameters in constructor, let getParameters() return the result of that), and maybe add their own specific tests.
• We can also set up additional generic tests for special signature scheme classes by extending the generic tests like SPSEQSchemeTest extends SignatureSchemeTest and adding a bunch of SPSEQ-specific @Test methods into SPSEQSchemeTest. Then any concrete SPSEQ construction's test can inherit from SPSEQSchemeTest.

Similarly for encryption schemes.

I see this as a "post release" thing (because it's a bit much to rework in a day), but either @rheitjoh or me should do this at some point.

Postscript

While doing this, maybe we can modernize the test parameter generation a bit, too. For example,

RingElementPlainText[] messages = new RingElementPlainText[numMessages];
for (int i = 0; i < messages.length; i++) {
    messages[i] = new RingElementPlainText(pp.getZp().getUniformlyRandomElement());
}
MessageBlock messageBlock = new MessageBlock(messages);

can now be much more succinctly expressed as

pp.getZp().getUniformlyRandomElements(numMessages).map(RingElementPlainText::new, MessageBlock::new);