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

Add --fail-fast to stop executing tests when if there are asserts error #3354

Open
jcamiel opened this issue Oct 29, 2024 · 6 comments
Open
Assignees
Labels
enhancement New feature or request

Comments

@jcamiel
Copy link
Collaborator

jcamiel commented Oct 29, 2024

When a Hurl file is in error, we stop the tests run as soon as we can (the default is "fail fast")

For instance:

$ hurl --jobs 1 --test a.hurl b.hurl c.hurl d.hurl

Given b.hurl is in error, executed tests will only be a.hurl and b.hurl.

We have an option called --continue-on-error that, if one Hurl file is in error, will try to execute this file until the end.

As the doc tells:

Note that this option does not affect the behavior with multiple input Hurl files.

All the input files are executed independently. The result of one file does not affect the execution of the other Hurl files.

So we need another option to run all Hurl files, regardless of there are error or not => --no-fail-fast.

@jcamiel jcamiel added the enhancement New feature or request label Oct 29, 2024
@krakenbite
Copy link

Hi,

I would like to work on this issue. Can I take this on?

@jcamiel
Copy link
Collaborator Author

jcamiel commented Nov 10, 2024

Hi @krakenbite

You can of course but be warned that this one can be more difficult that it seems.

For a starter, you can also work on issues labeled "good first issues" good first issue Good for newcomers

@jcamiel jcamiel self-assigned this Nov 13, 2024
@krakenbite
Copy link

@jcamiel Thanks for your response!

I already had a look at the code and I think I already found the right places which need modification.

As we currently bail-out on errors, question is how we want to handle errors in the future. How do we want to display them to the user if one or several files have issues?

Also, I think I can have a look at this issue as I like to challenge myself with difficult problems and don't need an issue labeled good first issue Good for newcomers .

@jcamiel
Copy link
Collaborator Author

jcamiel commented Nov 18, 2024

Hi @krakenbite

Trying to give you some examples, I'm realizing that we're alredy in "no fail fast" mode.
Given a.hurl, b.hurl, c.hurl ... m.hurl test files with every file being successful except c.hurl and d.hurl failing, we're running all the files until the end:

$ hurl --test --jobs 1 *.hurl
a.hurl: Success (1 request(s) in 157 ms)
b.hurl: Success (1 request(s) in 164 ms)
error: Assert status code
  --> c.hurl:2:6
   |
   | HEAD https://hurl.dev
 2 | HTTP 300
   |      ^^^ actual value is <200>
   |

c.hurl: Failure (1 request(s) in 166 ms)
error: Assert failure
  --> d.hurl:4:0
   |
   | GET https://hurl.dev
   | ...
 4 | xpath "string(//title)" == "foo"
   |   actual:   string <Hurl - Run and Test HTTP Requests>
   |   expected: string <foo>
   |

d.hurl: Failure (1 request(s) in 242 ms)
e.hurl: Success (1 request(s) in 162 ms)
f.hurl: Success (1 request(s) in 169 ms)
g.hurl: Success (1 request(s) in 172 ms)
h.hurl: Success (1 request(s) in 172 ms)
i.hurl: Success (1 request(s) in 165 ms)
j.hurl: Success (1 request(s) in 243 ms)
k.hurl: Success (1 request(s) in 168 ms)
l.hurl: Success (1 request(s) in 173 ms)
m.hurl: Success (1 request(s) in 180 ms)
--------------------------------------------------------------------------------
Executed files:    13
Executed requests: 13 (5.5/s)
Succeeded files:   11 (84.6%)
Failed files:      2 (15.4%)
Duration:          2378 ms

Note that we're talking about assertion failure, all the file are valid Hurl files. If one of the file is not syntaxically correct, we're stopping execution asap. We do not want to change the running behavior when some files are not valid Hurl files.

So, we can change the issue to support a "fail fast" mode, which is not the default. In this mode, running the previous test will display:

$ hurl --test --jobs 1 --fail-fast *.hurl
a.hurl: Success (1 request(s) in 157 ms)
b.hurl: Success (1 request(s) in 164 ms)
error: Assert status code
  --> c.hurl:2:6
   |
   | HEAD https://hurl.dev
 2 | HTTP 300
   |      ^^^ actual value is <200>
   |
--------------------------------------------------------------------------------
Executed files:    3
Executed requests: 3 (5.5/s)
Succeeded files:   2 (66.6%)
Failed files:      1 (33.3%)
Duration:          1200 ms

Note: this option doesn't affect existing --continue-on-error. With --continue-on-error, we keep running a file even if there is an error. It doesn't affect fail-fast/no-fail-fast (maybe the name of the --continue-on-error is not clear enough).

Example: if there are multiple errors in c.hurl

$ hurl --test --continue-on-error --fail-fast --jobs 1 *.hurl
a.hurl: Success (1 request(s) in 161 ms)
b.hurl: Success (1 request(s) in 163 ms)
error: Assert status code
  --> c.hurl:2:6
   |
   | HEAD https://hurl.dev
 2 | HTTP 300
   |      ^^^ actual value is <200>
   |

error: HTTP connection
  --> c.hurl:5:5
   |
 5 | GET https://goog.vom
   |     ^^^^^^^^^^^^^^^^ (6) Could not resolve host: goog.vom
   |

c.hurl: Failure (1 request(s) in 197 ms)
--------------------------------------------------------------------------------
Executed files:    3
Executed requests: 3 (5.5/s)
Succeeded files:   2 (66.6%)
Failed files:      1 (33.3%)
Duration:          1200 ms

Whereas without --continue-on-error:

$ hurl --test --fail-fast --jobs 1 *.hurl
a.hurl: Success (1 request(s) in 161 ms)
b.hurl: Success (1 request(s) in 163 ms)
error: Assert status code
  --> c.hurl:2:6
   |
   | HEAD https://hurl.dev
 2 | HTTP 300
   |      ^^^ actual value is <200>
   |

c.hurl: Failure (1 request(s) in 197 ms)
--------------------------------------------------------------------------------
Executed files:    3
Executed requests: 3 (5.5/s)
Succeeded files:   2 (66.6%)
Failed files:      1 (33.3%)
Duration:          1200 ms

(I'm changing the issue title from --no-fail-fast to --fail-fast)

@jcamiel jcamiel changed the title Add --no-fail-fast to keep executing tests even if there are error Add --fail-fast to stop executing tests when if there are asserts error Nov 18, 2024
@krakenbite
Copy link

Hi @jcamiel,

thank you very much for the extensive explanation. I now understand that I was thinking about the errors in a wrong way. Hence I suggest that we should not use only the term error but the more explicit variants file error (which covers things like bad syntax, file not found, etc.) and execution error (errors that are only related to the execution of a file like a non-reachable URL, an assertion error, etc.) to avoid future misconception of the term error. We can also use other terms for the variants if you have a better suggestion for them, this is what I could think of right now.

I have another question as you already brought up the flag --continue-on-error. This flag applies only to the single file mode and is ignored in the multi file mode of hurl. To be honest, I would be very confused as a user when reading about --continue-on-error and fail-fast options in the help/doc, especially as they only work in only a subset of the issued commands to hurl. IMO we could have a no-fail-fast option (which would be the default option to be compatible which older hurl versions), which does not fail hurl as soon as an execution error is encountered, and a fail-fast option, which would make hurl fail as soon as an execution error is encountered. Also, this could be used for both modes of hurl, making it a more seamless experience for the user (as the user does not have to remember that --continue-on-error applies for the single-file-mode, while fail-fast affects the multi file mode. Also, it would be a more concise and clean solution and would use the standard wording that is already established among other cli tools (which pays into better UX and therefore easier adoption for people that are already used to these terms).

Of course we would keep the --continue-on-error for now to make sure that this change will be backward compatible (and maybe deprecate --continue-on-error to remove it in a future release?). But we should then also think about what we want to do if both flags (--continue-on-error and [no]-fail-fast) are given.

@jcamiel
Copy link
Collaborator Author

jcamiel commented Nov 24, 2024

Hi @krakenbite

You're right about the distinction on errors. We use to speak about two types of errors:

  • syntax errors: when an Hurl file is not syntactically valid (in integration tests, see integration/hurl/tests_error_parser)
  • runtime errors: a file is valid, it can be run, and its runtime execution will either succeed or fail.

We don't want to change the behavior of Hurl with syntax errors. As soon as there is a syntax error, Hurl exits and we don't want, for the moment, add any option to keep going. This is mainly to avoid us to deal with some edge cases: for instance, if there is a syntax error, we don't have to manage the display of broken Hurl files in HTML reports, where the color highlighting is only working on valid Hurl files. One can also see it as if there is a kind of pre-compilation pass of all the input files.

There is also the case of the hurl crate. The signature of the main run function is:

pub fn run(
    content: &str,
    filename: Option<&Input>,
    runner_options: &RunnerOptions,
    variables: &VariableSet,
    logger_options: &LoggerOptions,
) -> Result<HurlResult, String> {
}

This function fails only if content is not a valid Hurl file. If it is a valid file, the function always returns successfully, with an HurlResult that can be successfull or not.

Now, regarding --fail-fast, --no-fail-fast, --continue-on-error. The difficulty is, as you said, to make something understandable and somewhat expected (we already struggle with the name, we've changed it from --fail-at-end to --continue-on-error).

What we have now is:

  • --no-fail-fast, --fail-fast: is about running multipleHurl files and what's happens when there is a runtime error in one of them
  • --continue-on-error: is about running the current file: if there is a runtime error, do you want to keep going or go to the next file.

Actually we have , by default: --no-fail-fast and -continue-on-error= no . We can't use the same name option because we're already doing two different things: if there is an error in the file, we don't continue (i.e. "failing fast inside a file"); if a file is in error, we execute the next one (i.e."no failing fast from file to file").

I completely agree it's confusing. Myself, I've to refer to the docs (and sometimes check the code) to remember what's doing what.
@fabricereix @lepapareil maybe we should remove completely --continue-on-error and just keep --fail-fast as option. I don't think --continue-on-error is that useful, regarding we have also --ignore-asserts 😅?

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

No branches or pull requests

2 participants