New PR for --print option without printing --actual results #24
Conversation
|
What's the problem with the tests? I use |
|
We run the tests with should have an extra space: |
|
Great @schoettl. Please go ahead and combine these into a single commit, I think it'll be easier for me in this case. |
tests/examples/cat.test
Outdated
| @@ -5,7 +5,6 @@ cat | |||
| foo | |||
| >>> | |||
| foo | |||
| >>>2 | |||
| >>>= 0 | |||
There was a problem hiding this comment.
The test no longer matches its description. Why remove the >>>2 ?
There was a problem hiding this comment.
I thought, it doesn't matter to remove the line. But you're right, it is less explicit. I removed the line, to have one more example file for shelltests of --print
| $$$ false | ||
| >>>= 1 | ||
|
|
||
| # no comment at begin of file because they are currently not parsed because of shared input |
There was a problem hiding this comment.
We should mention this limitation in docs. Where should we document --print ? In the CLI help and readme ?
There was a problem hiding this comment.
I'd document it in the CLI help and (not in this commit) in the readme. Because I think it's easier to write the doc when the bigger feature is ready (--actual-results)
There was a problem hiding this comment.
It's normally better to commit the doc along with the new feature being committed. We might forget, or there might be a delay in the next feature. At least, let's add the option to the --help output in README.
There was a problem hiding this comment.
How about this:
At least it's up to date then and don't have to be fixed manually.
There was a problem hiding this comment.
Good, thanks for updating that.
| -- running tests | ||
|
|
||
| testFileParseToHUnitTest :: Args -> Either ParseError [ShellTest] -> Test.HUnit.Test | ||
| testFileParseToHUnitTest args (Right ts) = TestList $ map (shellTestToHUnitTest args) ts | ||
| testFileParseToHUnitTest args (Right ts) = TestList $ map (\t -> testname t ~: prepareShellTest args t) ts | ||
| testFileParseToHUnitTest _ (Left e) = ("parse error in " ++ (sourceName $ errorPos e)) ~: (assertFailure :: (String -> IO ())) $ show e | ||
|
|
There was a problem hiding this comment.
This feels non-ideal, but it works. I don't see how exactly the types work - how does assertString become an IO action in the Nothing case ?
Perhaps we could describe it a little more clearly:
Convert this test to an IO action that either runs the test or prints it to stdout, depending on the arguments.
There was a problem hiding this comment.
assertString is IO: https://hackage.haskell.org/package/HUnit-1.6.0.0/docs/Test-HUnit-Base.html#t:Assertion
OK, I'll change the doc comment.
|
@simonmichael Should we add this section already to the readme? https://github.com/schoettl/shelltestrunner/tree/print#printing-tests I plan to add the print results feature soon. |
Sounds great! But please remove the mentions of future work, there's no need to include those in this PR. |
|
Looks good! Thanks a lot! |
@simonmichael for review.
There is now --print support for v1-3
I added tests in tests/print/
Specifics:
>=1instead of>= 1<<</>>>After review, I can squash the commits to one.