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

Fix dry run with messages #1540

Merged
merged 12 commits into from
May 18, 2021
Merged

Conversation

aurelien-reeves
Copy link
Contributor

Fixes #1496 and #1488
I've just applied the suggestion from @cbochs in #1496, it seems to work as expected.

It did not get well previously actually :(

@aurelien-reeves
Copy link
Contributor Author

So, it looks like the order of the filters is very important 😅

@aurelien-reeves aurelien-reeves changed the title Fix dry run with messages 1496 Fix dry run with messages May 11, 2021
@@ -10,6 +10,9 @@ name: Ruby
on:
push:
branches: [main, publish-option]
pull_request:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should not do this. We only want to publish reports for what's on the main branch.

I understand why you did this, and I think what we need instead is a unit or acceptance test that reproduces the problem that surfaces here when you run cucumber --publish.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am stuck trying to add an acceptance test which reproduce the issue

This is because the tested code is also the testing code. Because of that we cannot have acceptance tests based on the execution order of the filters in the runtime because it is actually executed a first time before executing the test itself.

At the moment I have not been able yet to write a spec either.

@mattwynne mattwynne added this to the 6.1.0 milestone May 12, 2021
@mattwynne
Copy link
Member

We decided that adding a test for this is too hard right now, as:

  1. It's impossible to use an acceptance test, as the Runtime is already loaded in the acceptance tests
  2. The formatter unit tests currently duplicate code from the runtime in a helper (see TODO added in 2667c93)

@aurelien-reeves aurelien-reeves merged commit cf4277d into main May 18, 2021
@aurelien-reeves aurelien-reeves deleted the fix-dry-run-with-messages-1496 branch May 18, 2021 06:13
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.

Using Cucumber CLI with "--dry-run" and "--format <message|html>" throws NilClass error
2 participants