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

[WIP] Print actual results with --print #22

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

schoettl
Copy link
Contributor

@schoettl schoettl commented Aug 8, 2020

Closes #21

CLI proposal

Print test file:
     --print[=FORMAT]   Print test files in specified format (default: v3).

[deferred:]
     --actual[=MODE]    Combined with --print, print test files with actual
                        results (stdout, stderr, exit status). Mode 'all'
                        prints all actual results (default). Mode 'update'
                        prints actual results only for non-matching results,
                        i.e. regular expressions in tests are retained.

TODO

  • Refactor printShellTest
  • Fix bug: type of expected result is not considered => regex are also put in the next line instead of directly after >=/>/>2
  • Support formats other than v3?
  • Fix parser to simply parse comments and blank lines without discarding the leading comment marker # or *.
  • Don't discard trailing comments after the last test
  • Create shelltests for --print
  • Print shared input as it was defined originally
  • shelltests for --print --actual
  • Don't discard comments at begin of file (before first test)

@schoettl
Copy link
Contributor Author

schoettl commented Aug 9, 2020

I need help the parsing of a comment line:

The parsed comment line discards the comment character (# or *).

I think it would be better to store the full comment line for this reasons:

  • When printing the shelltest file, I cannot restore the comment line without information about the comment character
  • The code for printing would be simpler (no case matching, just putStrLn comment)
  • The parsed commentline isn't used anywhere else anyway.

But I couldn't figure out how to change that line so that commentline stores the full line.

@simonmichael
Copy link
Owner

simonmichael commented Aug 18, 2020

Here's a rewrite of commentline that should work, though it could be made much more compact:

commentline = try (do
    prefix <- many1 (char '*') <|> (whitespace >> many1 (char '#'))
    rest <- lineoreof
    return $ prefix ++ rest
  ) <?> "commentline"

@schoettl
Copy link
Contributor Author

I know, this are a lot of changes already. I implemented your suggestion to parse commentline.

I also added a field trailingComments that are normally empty except for the last test. The alternative would be an extra data type only for trailing comments.

As far as I see, the biggest remaining problem is the "shared input". I don't even understand how the syntax works (haven't analyzed the parser yet). Currently, --print prints "shared input" as specific input for every test.

@simonmichael
Copy link
Owner

@schoettl I had a go at removing the actual output printing for now, but better to let you do it if possible. Would you mind removing just that part, and combining the rest into a single commit rebased on latest master ?

@simonmichael
Copy link
Owner

Oh shared input. It just means that after an input you can write more than one tests, and they'll all use it. Sorry for the hassle, but we will need to preserve this in the output.

@schoettl
Copy link
Contributor Author

@schoettl I had a go at removing the actual output printing for now, but better to let you do it if possible. Would you mind removing just that part, and combining the rest into a single commit rebased on latest master ?

Alright, I'll try to separate it.

@schoettl
Copy link
Contributor Author

Oh shared input. It just means that after an input you can write more than one tests, and they'll all use it. Sorry for the hassle, but we will need to preserve this in the output.

Yeah... that will mean more changes in the parser and probably two constructors for ShellTest.

Like data ShellTest = ShellTest { ... } | SharedInput { ... }

Do you agree?

@simonmichael
Copy link
Owner

two constructors for ShellTest

I don't have insight on that right now.. if you think it's the best option, let's give it a try.

@simonmichael
Copy link
Owner

I see what you mean, currently we don't model shared inputs in the type. If you want, that can be postponed for a future PR.

@schoettl
Copy link
Contributor Author

I see what you mean, currently we don't model shared inputs in the type. If you want, that can be postponed for a future PR.

Yup, I think that's a good idea.

@schoettl
Copy link
Contributor Author

I re-implemented the --actual options based on the --print already merged PR.

This feature requires --print option and prints an error if used without --print.

I'm open to changes about the option naming. Alternatives I considered:

  • --actual[=spec] because it prints actual results
  • --results[=spec] although it's very generic (test results, test reports, results of commands, ...)
  • --print-with[=spec] – the word print is redundant, when --print is already given. Using it instead of --print[=format] is also not a nice CLI because for me print is the main action.

FYI, the options around --print are in their own option group, see -h. So I think it doesn't matter if the new option doesn't contain the word "print".

I would add tests and doc, if this PR is a merge candidate.

@schoettl
Copy link
Contributor Author

The concept of this addition to --print is this:

Either non-matching-result matching-result

where result can be stdin/stdout/exit status.

From that and depending on the --actual[=spec], the Matcher is computed so that

  • it's the original result from the input shelltest file
  • it's the original Matcher where the test results match and the actual result where it doesn't – given spec = "update"
  • it's always the actual – given spec = "all"

@schoettl schoettl changed the title [WIP] Draft to print actual output [WIP] Print actual results with --print Aug 24, 2020
@simonmichael
Copy link
Owner

Blocker: #26

@schoettl
Copy link
Contributor Author

I will rebase when PR for #26 is merged.

@schoettl
Copy link
Contributor Author

The comments at begin of file are still not parsed. Thats because formatXtestgroup does skipMany whitespaceorcommentline before it parses many $ try (formatXtest i).

I don't quite understand this part of code. @simonmichael maybe you can look into it if you have time.

@simonmichael
Copy link
Owner

This is conflicting now - I had a look at resolving it but it's a bit much for me to tackle just at the moment, sorry.

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.

New option to print parsed test file with actual stdout, stderr, and exit status
2 participants