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

Upgrade to Cucumber 3.0 #6395

Merged
merged 3 commits into from Sep 29, 2017

Conversation

Projects
None yet
4 participants
@DirtyF
Member

DirtyF commented Sep 28, 2017

Tests are passing but the print_stats method is now undefined.

undefined method `print_stats' for #<Jekyll::Cucumber::Formatter:0x007fa12cbeba48> (NoMethodError)
/Users/frank/code/jekyll/jekyll/features/support/formatter.rb:210:in `print_summary'
/Users/frank/code/jekyll/jekyll/features/support/formatter.rb:49:in `after_features'

Breaking change introduced in cucumber/cucumber-ruby#999

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Sep 28, 2017

Member

cucumber/cucumber-ruby#999 (diff)

print_stats is now print_statistics

Member

pathawks commented Sep 28, 2017

cucumber/cucumber-ruby#999 (diff)

print_stats is now print_statistics

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Sep 28, 2017

Member

Fixes #6394

Member

pathawks commented Sep 28, 2017

Fixes #6394

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Sep 29, 2017

Member

@pathawks I thought we could go with the new summary format for now.
The only difference is that it doesn't print the time for each test.

Plus the following warning message implies that if we want to continue to customize the cucumber tests outputs, some refactoring will be necessary one day or another:

WARNING: The formatter Jekyll::Cucumber::Formatter is using the deprecated formatter API which will be removed in v4.0 of Cucumber.

Could we tackle this in another PR?

Member

DirtyF commented Sep 29, 2017

@pathawks I thought we could go with the new summary format for now.
The only difference is that it doesn't print the time for each test.

Plus the following warning message implies that if we want to continue to customize the cucumber tests outputs, some refactoring will be necessary one day or another:

WARNING: The formatter Jekyll::Cucumber::Formatter is using the deprecated formatter API which will be removed in v4.0 of Cucumber.

Could we tackle this in another PR?

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Sep 29, 2017

Member

Could we tackle this in another PR?

Yeah, let’s do. 👍

Member

pathawks commented Sep 29, 2017

Could we tackle this in another PR?

Yeah, let’s do. 👍

Show outdated Hide outdated script/cucumber Outdated
@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Sep 29, 2017

Member

Thanks @DirtyF.

I'm looking at using the new events API to get our timing information 👍

Member

pathawks commented Sep 29, 2017

Thanks @DirtyF.

I'm looking at using the new events API to get our timing information 👍

@pathawks pathawks added the dependency label Sep 29, 2017

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Sep 29, 2017

Member

@pathawks ❤️

Here is the summary display (very readable if you ask me)

cucumber-summary-jekyll

and the final summary:

212 scenarios (212 passed)
1763 steps (1763 passed)
4m16.614s
Member

DirtyF commented Sep 29, 2017

@pathawks ❤️

Here is the summary display (very readable if you ask me)

cucumber-summary-jekyll

and the final summary:

212 scenarios (212 passed)
1763 steps (1763 passed)
4m16.614s
@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Sep 29, 2017

Member

Wow.. nice..
Can you post a view for a failing test too..? (easily done locally..)

Member

ashmaroli commented Sep 29, 2017

Wow.. nice..
Can you post a view for a failing test too..? (easily done locally..)

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Sep 29, 2017

Member

also I think this leaves /features/support/formatter.rb redundant..

Member

ashmaroli commented Sep 29, 2017

also I think this leaves /features/support/formatter.rb redundant..

Show outdated Hide outdated script/cucumber Outdated
@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Sep 29, 2017

Member

also I think this leaves /features/support/formatter.rb reduntant..

No, I'm working on adding back some of that functionality. Let's keep it there.

Member

pathawks commented Sep 29, 2017

also I think this leaves /features/support/formatter.rb reduntant..

No, I'm working on adding back some of that functionality. Let's keep it there.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Sep 29, 2017

Member

cuc

Abbreviated to only show one group, but you can get the gist.

Member

pathawks commented Sep 29, 2017

cuc

Abbreviated to only show one group, but you can get the gist.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Sep 29, 2017

Member

I added a commit that will more closely resemble the previous output of Cucumber, but without using our own formatter.

This is just a first pass, and I tried to change as little as reasonable. Much of features/support/formatter.rb really could probably be removed, and I'm not sure if just sticking AfterConfiguration at the end is a good idea.

@DirtyF Feel free to change anything I added or just remove my commit altogether. Either way, I'm good upgrading to Cucumber 3.0 and we can tweak things as we go if need be.

Member

pathawks commented Sep 29, 2017

I added a commit that will more closely resemble the previous output of Cucumber, but without using our own formatter.

This is just a first pass, and I tried to change as little as reasonable. Much of features/support/formatter.rb really could probably be removed, and I'm not sure if just sticking AfterConfiguration at the end is a good idea.

@DirtyF Feel free to change anything I added or just remove my commit altogether. Either way, I'm good upgrading to Cucumber 3.0 and we can tweak things as we go if need be.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks
Member

pathawks commented Sep 29, 2017

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Sep 29, 2017

Member

@pathawks We'll discuss formatting in another PR. Thanks for your help on this. 🤝

@jekyllbot: merge +dev

Member

DirtyF commented Sep 29, 2017

@pathawks We'll discuss formatting in another PR. Thanks for your help on this. 🤝

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 7333baf into master Sep 29, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot deleted the cucumber-3.0 branch Sep 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment