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

Book / Docs / Readme teach using Cucumber.run() - which does not make cargo test fail with non-zero exit code #334

Open
Artalus opened this issue Apr 27, 2024 · 2 comments
Labels
bug Something isn't working k::documentation Related to project documentation

Comments

@Artalus
Copy link

Artalus commented Apr 27, 2024

#84 brought a valid topic that using run() did not allow you to see if the Cucumber tests fail. The output of this ticket was new run_and_exit() method - which, unlike run(), will make your executable panic and exit with non-zero code.

However, the Book - together with Docs.rs and Readme - never mentions that .run() still has this behavior and that you can avoid it. The Book is still using the new method, but a-matter-of-fact'ly - some snippets just call .run_and_exit() instead of .run() without drawing any attention to it. This has led me to quite a surprise today when I noticed that my automation was not picking up some failures in Cucumber tests, since the executable using .run() was always exiting with 0, and so the cargo test command never failed technically 🙂

My suggestion would be to

  • either simply mass-replace every use of .run( with .run_and_exit(;
  • or at least have a plaque right in Quickstart part of the Book, stating the difference between the two, if .run() is considered to be of some value.
@ilslv
Copy link
Member

ilslv commented Apr 27, 2024

To be fair, I don't quite understand what you are suggesting.

either simply mass-replace every use of .run( with .run_and_exit(;

As you mentioned earlier, book already uses .run_and_exit() for more complex setups.

or at least have a plaque right in Quickstart part of the Book, stating the difference between the two, if .run() is considered to be of some value.

Yes, Cucumber::run is still useful for example for internal tests. Instead of panicking, Writer can collect some info and then assertion can be made based on its content. About mentioning it in the quickstart guide, I don't think it's useful, as docs already have Panics section for every function of the public API that can panic.

@Artalus
Copy link
Author

Artalus commented Apr 27, 2024

As you mentioned earlier, book already uses .run_and_exit() for more complex setups.

Yes, but it just... does it. Silently. I admit I skimmed over the later parts of the book instead of reading them carefully - but I did not even notice the change in the methods used.

Yes, Cucumber::run is still useful for example for internal tests. Instead of panicking, Writer can collect some info and then assertion can be made based on its content.

Thank you. That is a nice thing to have exposed in the API indeed! But:

About mentioning it in the quickstart guide, I don't think it's useful

I disagree. The quickstart makes it look like "to run the Cucumber tests and be all settled, you only call .run()". It does not mention that you need to do something with the result of .run() to make your test executable actually fail with your tests. It does not even indicate that .run() returns a meaningful result you can use later on. I haven't looked into docs until I encountered my problem but I was sure it returns () - otherwise the book would mention it?..

To be fair, I don't quite understand what you are suggesting.

I am suggesting that the "default" way to run the tests - taught by the book in its very first "how to" chapter - should involve exiting with non-zero code on test failure. Especially if they are intended to be a part of cargo test suite. Otherwise you get a nice red output from cargo test, but for example your CI status would still be green.


Interesting enough, my results seem to differ from the book. In the first picture of a failing test there are both thread 'main' panicked and error: test failed, to rerun pas --test example lines. Yet if I run the code from quickstart my tests seem to pass from cargo perspective:

Artalus@ArtPC $ cargo test --test example
     Running tests\example.rs (target\debug\deps\example-ccc7c40af0e2c213.exe)
Feature: Animal feature
  Scenario: If we feed a hungry cat it will no longer be hungry
   ✔  Given a hungry cat
   ✔  When I feed the cat
   ✘  Then the cat is not hungry
      Step failed:
      Defined: ?\D:\rust\tests\features\animal.feature:6:5   
      Matched: tests\example.rs:36:1
      Step panicked. Captured output: assertion failed: world.cat.hungry
[Summary]
1 feature
1 scenario (1 failed)
3 steps (2 passed, 1 failed)

Artalus@ArtPC $ echo $?
0

@tyranron tyranron added bug Something isn't working k::documentation Related to project documentation labels Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working k::documentation Related to project documentation
Projects
None yet
Development

No branches or pull requests

3 participants