Remove theme directory confirmation during tests and make confirmation dialogue respect SHOPIFY_CLI_TTY

[Poitrin]

WHY are these changes introduced?

Fixes #1136

Issue 1: @ctx.testing? is true even outside a testing environment

CLI was hanging when theme pull was executed in a directory with already existing files.
CLI should have shown question

It doesn’t seem like you’re running this command in a theme directory. Are you sure you want to proceed?

but was never shown, due to

raise "Current theme directory can't be confirmed during tests" if @ctx.testing?

Discussion why raise … if @ctx.testing? was introduced can be found here. Because @ctx.testing? returns true based on ENV["CI"], we can’t differentiate between CLI running in partner’s pipeline or in CLI tests.

Issue 2: Confirmation dialogue ignores environment variable SHOPIFY_CLI_TTY

Moreover, even after having removed raise … if @ctx.testing? and SHOPIFY_CLI_TTY being set to 0, CLI would still hang at the “It doesn’t seem like …” question.

WHAT is this pull request doing?

1. Removes raise … if @ctx.testing?.
2. Shows a warning It doesn’t seem like you’re running this command in a theme directory. when SHOPIFY_CLI_TTY is set to 0, but continues to pull.
   • --force flag as a way to confirm the command will not be included in the error message yet.
     • → Validate on shopify theme share/pull/push/serve if users are running the command in a theme/empty directory shopify-cli#2680 (comment)
     • → [Enhancement] Introduce the -f/--force flag as a pattern for confirmation dialogues in the whole CLI #760

How to test your changes?

1. mkdir ./test-folder
2. cd ./test-folder
3. touch randomfile1.txt randomfile2.log
4. cd ..
5. Retrieve a random theme ID from shopify theme list
6. Run CI=true pnpm shopify:run theme pull --theme <Your Theme ID> --path="./test-folder".
   • Verify that warning It doesn’t seem like you’re running this command in a theme directory. and confirmation dialogue Are you sure you want to proceed? are shown.
   • Ctrl+C.
7. Run SHOPIFY_CLI_TTY=0 CI=true pnpm shopify:run theme pull --theme <Your Theme ID> --path="./test-folder".
   • Verify that CLI displays warning It doesn’t seem like you’re running this command in a theme directory. but continues to pull.
   • Ctrl+C the pull and make sure that your test-folder is a non-theme directory (again).
8. Run CI=true pnpm shopify:run theme pull --theme <Your Theme ID> --path="./test-folder" --force.
   • Verify that neither warning nor confirmation dialogue are shown and that theme files get pulled.

Measuring impact

How do we know this change was effective? Please choose one:

• n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

• I've considered possible cross-platform impacts (Mac, Linux, Windows)
• I've considered possible documentation changes
• I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.

[github-actions]

Coverage report

St.❔	Category	Percentage	Covered / Total
🟡	Statements	73.39%	4168/5679
🟡	Branches	70.48%	1874/2659
🟡	Functions	71.17%	1096/1540
🟡	Lines	74.69%	3980/5329

Test suite run success

1052 tests passing in 536 suites.

Report generated by 🧪jest coverage report action from 567c9f8

[mgmanzella]

noice 🔥 thank you @Poitrin!

[github-actions]

Benchmark report

The following table contains a summary of the startup time for all commands.

Status	Command	Baseline (avg)	Current (avg)	Diff
🟢	app build	926.33 ms	930.33 ms	0.43 %
🟢	app deploy	1114.67 ms	1105.67 ms	-0.81 %
🟢	app dev	1106.33 ms	1114.33 ms	0.72 %
🟢	app env pull	1026.67 ms	1005.33 ms	-2.08 %
🟢	app env show	1011.33 ms	1033.33 ms	2.18 %
🟢	app function build	924 ms	916.33 ms	-0.83 %
🟢	app function run	907.33 ms	912.67 ms	0.59 %
🟢	app function schema	1041 ms	1035.67 ms	-0.51 %
🟢	app function typegen	910.67 ms	914.67 ms	0.44 %
🟢	app generate extension	1072.33 ms	1078.67 ms	0.59 %
🟢	app generate schema	1040.33 ms	1064.33 ms	2.31 %
🟢	app info	1017.67 ms	1024.33 ms	0.66 %
🟢	app scaffold extension	1079 ms	1080.33 ms	0.12 %
🟢	app update-url	988.67 ms	996.33 ms	0.78 %
🟢	theme check	865 ms	857 ms	-0.92 %
🟢	theme console	975.67 ms	964.67 ms	-1.13 %
🟢	theme delete	969.67 ms	973.33 ms	0.38 %
🟢	theme dev	977.33 ms	974.33 ms	-0.31 %
🟢	theme help-old	862 ms	860.33 ms	-0.19 %
🟢	theme info	903.67 ms	915 ms	1.25 %
🟢	theme init	882.33 ms	873.67 ms	-0.98 %
🟢	theme language-server	866.33 ms	856.33 ms	-1.15 %
🟢	theme list	975 ms	984 ms	0.92 %
🟢	theme open	977.33 ms	976.33 ms	-0.1 %
🟢	theme package	911 ms	913 ms	0.22 %
🟢	theme publish	973 ms	967 ms	-0.62 %
🟢	theme pull	976 ms	969.67 ms	-0.65 %
🟢	theme push	971 ms	969.33 ms	-0.17 %
🟢	theme serve	980.67 ms	973.67 ms	-0.71 %
🟢	theme share	973.67 ms	957 ms	-1.71 %
🟢	webhook trigger	993 ms	987.33 ms	-0.57 %

[Poitrin]

@karreiro suggested to replace the abort with a warn, which sounds like a better approach, especially as the error message does not show any way to --force the pull.
@mgmanzella – can you take a 2ⁿᵈ look again? :)

[karreiro]

Thank you, @Poitrin!