Try to fix failures related to changes in Thor 1.3.0 #913

davidstosik · 2023-10-31T00:49:11Z

Update: please take a look at this alternative: #914. I think it's a better approach to the issue.

The release of Thor 1.3.0 introduced issues related to:

how Thor determines the terminal width (the #terminal_width method moved) (Extract print methods to seperate classes rails/thor#854)
whether Thor wraps and indents a long description, or not (🌧️ long_desc: new option to disable wrapping rails/thor#739)

This PR should allow tests to pass whether Thor's version is before or after these changes:

$ rake test
# success

$ bundle update thor
# ...
# Installing thor 1.3.0 (was 1.2.2)
# ...

$ rake test
# success

slack-github-archiver · 2023-10-31T00:49:32Z

Summary

Nikita Vasilevsky reported a failing test on the maintenance_tasks gem CI, related to terminal_width in a version of Thor.
David Stosik suggested setting the THOR_COLUMNS environment variable to force Thor to work at a given terminal width.
David confirmed that Thor::Base.shell.new.terminal_width used to work in 1.2.2 but now fails in the latest 1.3.0. This change was not mentioned in the release notes.
David proposed two solutions: filing a bug report on Thor for regression, or stubbing different methods depending on the version of Thor.
David also found several repositories setting THOR_COLUMNS in tests as a potential solution.

This summary was generated using OpenAI's gpt-4 with a temperature of 0.5.

🧵 Slack Thread

User		Message
	^{Nikita Vasilevsky 2023‑10‑30 14:34}	hey @davidstosik, there is one test that is failing on `maintenance_tasks` gem CI (example) which is related to the `terminal_width` not existing in a newer/older version of `Thor`. I haven’t looked too deep to be honest but I was wondering if perhaps you have a quick fix in mind since you’ve spent some time exploring alternatives - https://github.com/Shopify/[…]files#r1310016162 It’s okay if you don’t have a fix off the top of your head, we can look into it! Just wanted to avoid exploring something that has potentially been discussed already. Thanks!
	^{David Stosik 2023‑10‑30 23:53}	Hello Nikita! That rings a bell indeed. Let me have a quick look.
	^{David Stosik 2023‑10‑31 00:00}	It looks like one might be able to set the `THOR_COLUMNS` environment variable to force Thor to work at a given terminal width
	^{David Stosik 2023‑10‑31 00:05}	That env var was introduced almost 14 years ago, so I’d be confident that acting on that would impact all Thor versions that matter. I’m surprised about the error with `terminal_width` though, because it was made part of the public API 12 years ago, and might have been unceremoniously removed very recently. I think it would be good to know what version of Thor is failing, and whether that could be a breaking change in Thor that needs to be addressed.
	^{David Stosik 2023‑10‑31 00:23}	At least I confirmed that `Thor::Base.shell.new.terminal_width` used to work in `1.2.2` but now fails in the latest `1.3.0`. This change was not mentioned in the release notes. (Even though that was public API for the longest time.) 1. I wonder if this should be a bug report on Thor. (regression?) 2. You could stub different methods depending on the version of Thor: either `Thor::Shell::Terminal.stubs(:terminal_width)` after 1.3.0 or `Thor::Base.shell.any_instance.stubs(:terminal_width)` before Something like: "thor/version" method_owner = if Thor::VERSION >= "1.3.0" require "thor/shell/terminal" Thor::Shell::Terminal else Thor::Base.shell.any_instance end method_owner.stubs(:terminal_width).returns(200) I think setting the `THOR_COLUMNS` env var (globally for the whole test suite?) might be a better approach though
	^{David Stosik 2023‑10‑31 00:27}	I found quite a few repos setting `THOR_COLUMNS` in tests, like this: # Don't wrap output in tests ENV["THOR_COLUMNS"] = "10000"

David Stosik archived this conversation from team-rails-infra at 2023‑10‑31 00:49.
All times are in UTC.

davidstosik · 2023-10-31T00:56:26Z

test/test_helper.rb

@@ -3,6 +3,9 @@
 # Configure Rails Environment
 ENV["RAILS_ENV"] = "test"

+# Don't wrap output in tests
+ENV["THOR_COLUMNS"] = "200"


I confirmed this had the expected consequence for both Thor 1.2.2 and Thor 1.3.0:

require "thor/version" puts Thor::VERSION if Thor::VERSION >= "1.3.0" assert_equal 200, Thor::Shell::Terminal.terminal_width else assert_equal 200, Thor::Base.shell.new.terminal_width end

Unfortunately, the output is still wrapped when using Thor 1.3.0:

Failure: MaintenanceTasks::CLITest#test_`help_perform`_loads_all_tasks_and_displays_them [test/lib/maintenance_tasks/cli_test.rb:104]: In stdout. Expected /\ \ `maintenance_tasks\ perform`\ will\ run\ the\ Maintenance\ Task\ specified\ by\ the\ \[TASK\ NAME\]\ argument\.\n\n\ \ Available\ Tasks:\n\n\ \ Task1\n\n\ \ Task2\n/ to match "Usage:\n cli_test.rb perform [TASK NAME]\n\nOptions:\n [--csv=CSV] # Supply a CSV file to be processed by a CSV Task, --csv path/to/csv/file.csv\n [--arguments=key:value] # Supply arguments for a Task that accepts parameters as a set of <key>:<value> pairs.\n\nDescription:\n`maintenance_tasks perform` will run the Maintenance Task specified by\nthe [TASK NAME] argument.\n\nAvailable Tasks:\n\nTask1\n\nTask2\n".

Cleaning up, we have:

Failure: MaintenanceTasks::CLITest#test_`help_perform`_loads_all_tasks_and_displays_them [test/lib/maintenance_tasks/cli_test.rb:104]: In stdout. Expected / `maintenance_tasks perform` will run the Maintenance Task specified by the [TASK NAME] argument. Available Tasks: Task1 Task2 / to match " Usage: cli_test.rb perform [TASK NAME] Options: [--csv=CSV] # Supply a CSV file to be processed by a CSV Task, --csv path/to/csv/file.csv [--arguments=key:value] # Supply arguments for a Task that accepts parameters as a set of <key>:<value> pairs. Description: `maintenance_tasks perform` will run the Maintenance Task specified by the [TASK NAME] argument. Available Tasks: Task1 Task2 ".

Thor CLI output was cut after specified by (70 characters), even though Thor::Shell::Terminal.terminal_width returns 200.

The Thor CLI output also seems to be missing indentation.

I confirmed the CLI output changed:

Before

Usage: maintenance_tasks perform [TASK NAME] Options: [--csv=CSV] # Supply a CSV file to be processed by a CSV Task, --csv path/to/csv/file.csv [--arguments=key:value] # Supply arguments for a Task that accepts parameters as a set of <key>:<value> pairs. Description: `maintenance_tasks perform` will run the Maintenance Task specified by the [TASK NAME] argument. Available Tasks: Maintenance::BatchImportPostsTask Maintenance::CallbackTestTask etc...

After

cd test/dummy && ../../exe/maintenance_tasks help perform Usage: maintenance_tasks perform [TASK NAME] Options: [--csv=CSV] # Supply a CSV file to be processed by a CSV Task, --csv path/to/csv/file.csv [--arguments=key:value] # Supply arguments for a Task that accepts parameters as a set of <key>:<value> pairs. Description: `maintenance_tasks perform` will run the Maintenance Task specified by the [TASK NAME] argument. Available Tasks: Maintenance::BatchImportPostsTask Maintenance::CallbackTestTask etc...

The description is now cut at 70 at not indented.

davidstosik · 2023-10-31T01:17:02Z

lib/maintenance_tasks/cli.rb

@@ -56,6 +56,8 @@ def perform(name)
      LONGDESC
    end

+    commands["perform"].wrap_long_description = true


This ensures newer versions of Thor (>= 1.3.0, including rails/thor#739) will wrap and indent the long description as older versions used to.

It fixes the problem I'm describing in this other comment thread.

Try to fix terminal_width changes in Thor 1.3.0

b2afbef

davidstosik commented Oct 31, 2023

View reviewed changes

davidstosik force-pushed the sto/terminal_width branch from 677badc to e517c4c Compare October 31, 2023 01:15

davidstosik commented Oct 31, 2023

View reviewed changes

Make sure Thor >=1.3.0 will wrap the perform command's long description

8cc0fe6

davidstosik force-pushed the sto/terminal_width branch from e517c4c to 8cc0fe6 Compare October 31, 2023 01:24

davidstosik changed the title ~~Try to fix terminal_width changes in Thor 1.3.0~~ Try to fix failures related to changes in Thor 1.3.0 Oct 31, 2023

davidstosik requested a review from nvasilevski October 31, 2023 03:02

davidstosik mentioned this pull request Oct 31, 2023

Introduce list command in CLI #914

Merged

davidstosik closed this in #914 Nov 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try to fix failures related to changes in Thor 1.3.0 #913

Try to fix failures related to changes in Thor 1.3.0 #913

davidstosik commented Oct 31, 2023 •

edited

Loading

slack-github-archiver bot commented Oct 31, 2023

davidstosik Oct 31, 2023

davidstosik Oct 31, 2023

davidstosik Oct 31, 2023 •

edited

Loading

Try to fix failures related to changes in Thor 1.3.0 #913

Try to fix failures related to changes in Thor 1.3.0 #913

Conversation

davidstosik commented Oct 31, 2023 • edited Loading

slack-github-archiver bot commented Oct 31, 2023

Summary

davidstosik Oct 31, 2023

Choose a reason for hiding this comment

davidstosik Oct 31, 2023

Choose a reason for hiding this comment

Before

After

davidstosik Oct 31, 2023 • edited Loading

Choose a reason for hiding this comment

davidstosik commented Oct 31, 2023 •

edited

Loading

davidstosik Oct 31, 2023 •

edited

Loading