Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation of the Cram tests using Exactly #398

Open
wants to merge 2 commits into
base: bleeding
Choose a base branch
from

Conversation

emilkarlen
Copy link

Adds an implementation of the Cram tests using Exactly (https://github.com/emilkarlen/exactly).

NOTE: This PR does not replace the Cram tests - so it should probably not be merged as is.

Rather, this is a place for discussing which test tool is best suited for the project, as suggested by @Snaipe.

If the conclusion is that Exactly should replace Cram, then I will remove the Cram tests.

See my evaluation in https://github.com/emilkarlen/Criterion-exactly (README)

@Snaipe
Copy link
Owner

Snaipe commented Dec 21, 2021

Looking through this, this seems fairly nice, especially the builtin text processing where normally one would have to use sed or something else in the cram test to manipulate the program output.

The failmessage substitution for IEEE double and long double floating-point number representations is mostly the result of laziness in our current test suite; the right way would have been to have meson detect the right representation for the platform under test and tell cram or exactly which one to use, rather than testing both forms. The problem of either form matching and causing a false positive is not a problem in Exactly since it exists in our Cram tests today.

One thing I've really been missing in Cram today is the inability to run all of the tests concurrently, which is caused by a design flaw in Cram itself (since it mandates commands to be executed sequentially within a test file). While I understand this is not something that Exactly can do today, its design seem to at least make this possible at some point in the future.

One very important feature that we use during development is cram -i, which patches our test files with the new expected output. This is especially important for larger changes that impact large portions of the output, since editing all of the tests by hand is not exactly practical. I haven't seen anything touching that specific aspect in Exactly's documentation, is this something that can be achieved today? I think that would be the biggest showstopper right now for us to move from Cram.

@emilkarlen
Copy link
Author

Bon Noël!

Concurrent execution

Concurrency is a feature that should be implemented, at some point,
definitely.
It has not been priorotized though - there are some
weaknesses that need to be fixed first.
But if it is important, I may take a closer look at it.

Automatic patching

This cannot be achieved today. And unfortunately, I have no good idea of how
this could be implemented, except in very special circumstances.

Some reasons:

  • Cram: checking of stdout is mandatory. Exactly: it isn't.

  • Exactly: expected contents may be in the test case file or in an external
    file.

  • Exactly: whole or parts of stdout/err may be checked.

    stdout -transformed-by
        grep "interesting"
        num-lines == 2
    
  • Exactly: multiple assertions may check different parts of stdout/err.

    stdout any line : contents matches "1st pattern"
    
    stdout any line : contents matches "2nd pattern"
    

Because of this, the patching feature do not feel natural for Exactly. Adding
it would require changes to the design - adding complexity - that would be
usefull only in very special circumastances. And thus hard to motivate.

On the other hand, the actual output can easily be retrieved:

exactly --act test.case

This could be used to accomplish the patching via scripts, using sed and
expected contents in external files.
But this could be a bit complicated, I believe.

So if the patching feature is important I believe that Cram is more suitable
than Exactly for this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants