Skip to content

Bump cucumber to v9#9747

Merged
jekyllbot merged 4 commits intojekyll:masterfrom
ashmaroli:cucumber-v9
Jan 16, 2025
Merged

Bump cucumber to v9#9747
jekyllbot merged 4 commits intojekyll:masterfrom
ashmaroli:cucumber-v9

Conversation

@ashmaroli
Copy link
Copy Markdown
Member

@ashmaroli ashmaroli commented Jan 5, 2025 •
edited
Loading

Summary

  • Bump gem cucumber to 9.x
  • Refactor features/support/formatter.rb
    • Be compatible with Cucumber v9.
    • Truncate printed scenario names to improve readability.
    • Remove unused code (since existing script/cucumber calls upon the progress formatter via --format progress).
    • Avoid printing scenario info when using the default pretty formatter (duplication / redundant).

@ashmaroli ashmaroli marked this pull request as ready for review January 15, 2025 15:30
@ashmaroli
Copy link
Copy Markdown
Member Author

ashmaroli commented Jan 15, 2025

Hello @parkr, would like to know if you have any misgivings about changes here since this would need to be backported to
3.9-stable branch as well eventually as Cucumber 9.2.1 is needed to test with Ruby 3.4+

@ashmaroli ashmaroli requested a review from parkr January 15, 2025 15:35
parkr
parkr reviewed Jan 15, 2025
View reviewed changes
features/support/formatter.rb
@@ -1,222 +1,80 @@
# frozen_string_literal: true
Copy link
Copy Markdown
Member

@parkr parkr Jan 15, 2025

Choose a reason for hiding this comment

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

Is this formatter still in use? Should we remove it if we always use progress, or use it in CI?

Copy link
Copy Markdown
Member Author

@ashmaroli ashmaroli Jan 16, 2025

Choose a reason for hiding this comment

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

Yes, the custom formatter is still used regardless of how one invokes Cucumber, i.e. bash script/cucumber foo.feature or script/cucumber foo.feature:24 or bundle exec cucumber foo.feature.

The advantage of custom formatter over the built-in progress formatter is that with our formatter, we print scenario location and name alongwith step-run-status subsequently, followed by a "Worst Offenders" section at the end. The built-in progress formatter would just print step-run-status in a single line stream, very similar to how RuboCop prints or Minitest prints.

Copy link
Copy Markdown
Member

@parkr parkr Jan 16, 2025

Choose a reason for hiding this comment

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

Ok, thanks!

features/support/formatter.rb

module Jekyll
module Cucumber
class Formatter
Copy link
Copy Markdown
Member

@parkr parkr Jan 15, 2025

Choose a reason for hiding this comment

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

Since there's no test for the formatter, can you share sample output for this so we can see what it looks like? I'm a little worried about truncation causing confusion about what scenario is described in the output so it would be beneficial to see. Thanks!

Copy link
Copy Markdown
Member Author

@ashmaroli ashmaroli Jan 16, 2025

Choose a reason for hiding this comment

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

You can see the sample output in our CI workflow logs for this pull request and compare with existing prints for the counterpart in the master branch.

@parkr
Copy link
Copy Markdown
Member

parkr commented Jan 16, 2025

No significant worries, just want to make sure the formatter we write here is tested in some fashion (e.g. by being used in our CI). It sounds like it is so we're good.

@ashmaroli
Copy link
Copy Markdown
Member Author

ashmaroli commented Jan 16, 2025 •
edited
Loading

Thanks for reviewing, Parker.
Posting a snippet from the two logs for posterity:

from the master branch

features/collections.feature:148  Scenario: Access rendered and published collection documents via Liquid .......................... (0.993368848s)
features/collections.feature:186  Scenario: Unrendered collection with future dated document ............. (0.983786877s)
features/collections.feature:209  Scenario: Access unrendered collection with future dated document via Liquid .............. (0.987088356s)
features/collections.feature:233  Scenario: Access unrendered but publishable collection documents via Liquid .......................... (0.99039234s)

[-- snip --]

Worst offenders:
  2.416956752s for "Include a file-path with non-alphanumeric character sequences" (features/include_tag.feature:108)
  2.060417055s for "A themed-site and incremental regeneration" (features/incremental_rebuild.feature:89)
  1.987070361s for "Rebuild when a dependency of document in custom collection_dir is changed" (features/incremental_rebuild.feature:70)
  1.985617546s for "Include a file and rebuild when include content is changed" (features/include_tag.feature:86)
  1.969061581s for "Rebuild when layout is changed" (features/incremental_rebuild.feature:40)

[-- snip --]

from head branch

features/collections.feature:148  "Access rendered and published collectio..."  ..........................  (1.347s)
features/collections.feature:186  "Unrendered collection with future dated..."  .............  (1.252s)
features/collections.feature:209  "Access unrendered collection with futur..."  ..............  (1.227s)
features/collections.feature:233  "Access unrendered but publishable colle..."  ..........................  (1.272s)

[-- snip --]

Worst offenders:
  features/include_tag.feature:108         "Include a file-path with non-alphanumer..." (2.800s)
  features/incremental_rebuild.feature:89  "A themed-site and incremental regenerat..." (2.311s)
  features/incremental_rebuild.feature:70  "Rebuild when a dependency of document i..." (2.203s)
  features/incremental_rebuild.feature:27  "Rebuild when content is changed"            (2.193s)
  features/incremental_rebuild.feature:40  "Rebuild when layout is changed"             (2.191s)

[-- snip --]

@ashmaroli
Copy link
Copy Markdown
Member Author

ashmaroli commented Jan 16, 2025

@jekyllbot: merge +dev

1 similar comment
@ashmaroli
Copy link
Copy Markdown
Member Author

ashmaroli commented Jan 16, 2025

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 2db7db7 into jekyll:master Jan 16, 2025
jekyllbot added a commit that referenced this pull request Jan 16, 2025
@jekyllbot
Update history to reflect merge of #9747 [ci skip]
4090344
@ashmaroli ashmaroli deleted the cucumber-v9 branch January 16, 2025 06:19
@ashmaroli ashmaroli added the backport-candidate Consider for merge into an older stable branch label Jan 16, 2025
ashmaroli added a commit to ashmaroli/jekyll that referenced this pull request Jan 16, 2025
@ashmaroli
Backport jekyll#9747 for v4.3.x: Bump cucumber to v9
f6f5497 
This backports 2db7db7 to 4.3-stable
ashmaroli added a commit that referenced this pull request Jan 16, 2025
@ashmaroli
Backport #9747 for v4.3.x: Bump cucumber to v9 (#9752)
a60cf32 
This backports 2db7db7 to 4.3-stable
@jekyll jekyll locked and limited conversation to collaborators Jan 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@parkr parkr parkr approved these changes

Assignees

No one assigned

Labels

backport-candidate Consider for merge into an older stable branch fix frozen-due-to-age internal

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ashmaroli @parkr @jekyllbot