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.

Sign up for GitHub

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

Jump to bottom

chore: upgrade tap@21 #8085

Draft
wants to merge 22 commits into
base: latest
Choose a base branch
from
Draft

chore: upgrade tap@21 #8085

wants to merge 22 commits into from

Conversation

reggi
Copy link
Contributor

@reggi reggi commented Feb 5, 2025 •
edited
Loading

apart of: npm/statusboard#913

Outline of Changes (all of these don't need to be in this PR and can be broken down individually)

  1. updates tap dep to 21
  2. Tap changed t.mock to t.mockRequire
  3. updates template-oss to change tap.exclude in package.json
  4. At the time of this PR vendored node_modules is dirty and needs updates from npa (should be in another pr)
  5. Discovery that tap@21 needs ./node_modules/minipass/dist/commonjs/index.js.map checked in to run correctly
  6. Including all .map files within node_modules, currently we don't check in .md, .ts, and .map (should be in another pr)
  7. Update all snapshots to the new format
  8. Custom file fixes
    • test/lib/cli/update-notifier.js
    • test/lib/commands/diff.js
    • test/lib/commands/doctor.js
    • test/lib/commands/exec.js
    • test/lib/commands/explore.js
    • test/lib/commands/link.js
    • test/lib/commands/ls.js
    • test/lib/commands/run-script.js
    • test/lib/utils/log-file.js
  9. Move on to workspaces
  10. Ensure we still maintain 100% coverage

@reggi reggi changed the title Reggi/tap ugprade 21 chore: upgrade tap@21 Feb 7, 2025
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 12, 2025
View reviewed changes
test/lib/commands/explore.js
@@ -70,7 +70,7 @@

t.ok(PJ_CALLED.endsWith('/pkg'))
t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
t.match(output, /Exploring \{CWD\}\/[\w-_/]+\nType 'exit' or \^D when finished/)
t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)

Check failure

Code scanning / CodeQL

Inefficient regular expression High test

This part of the regular expression may cause exponential backtracking on strings starting with 'Exploring {CWD}/' and containing many repetitions of 'a'.

Copilot Autofix AI 9 days ago

To fix the problem, we need to modify the regular expression to remove the ambiguity that causes exponential backtracking. Specifically, we can replace the .+ pattern with a more specific pattern that avoids ambiguity. In this case, we can use [^/]+ to match one or more characters that are not a forward slash, which aligns with the intended use of the regular expression.

  • Modify the regular expression on line 73 to use [^/]+ instead of .+.
  • Ensure that the new pattern still matches the intended strings without causing performance issues.
Suggested changeset 1
test/lib/commands/explore.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/test/lib/commands/explore.js b/test/lib/commands/explore.js
--- a/test/lib/commands/explore.js
+++ b/test/lib/commands/explore.js
@@ -72,3 +72,3 @@
   t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
-  t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
+  t.match(output, /Exploring \{CWD\}\/([^/]+)+\nType 'exit' or \^D when finished/)
 })
EOF
@@ -72,3 +72,3 @@
t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
t.match(output, /Exploring \{CWD\}\/([^/]+)+\nType 'exit' or \^D when finished/)
})
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
test/lib/commands/explore.js
@@ -83,7 +83,7 @@

t.ok(PJ_CALLED.endsWith('/pkg'))
t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
t.match(output, /Exploring \{CWD\}\/[\w-_/]+\nType 'exit' or \^D when finished/)
t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)

Check failure

Code scanning / CodeQL

Inefficient regular expression High test

This part of the regular expression may cause exponential backtracking on strings starting with 'Exploring {CWD}/' and containing many repetitions of 'a'.

Copilot Autofix AI 9 days ago

To fix the problem, we need to modify the regular expression to remove the nested quantifiers that cause exponential backtracking. The best way to do this is to replace (.+)+ with a more specific pattern that avoids ambiguity. In this case, we can use ([^/]+) to match one or more characters that are not a forward slash, which achieves the same goal without the risk of catastrophic backtracking.

Suggested changeset 1
test/lib/commands/explore.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/test/lib/commands/explore.js b/test/lib/commands/explore.js
--- a/test/lib/commands/explore.js
+++ b/test/lib/commands/explore.js
@@ -72,3 +72,3 @@
   t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
-  t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
+  t.match(output, /Exploring \{CWD\}\/([^/]+)\nType 'exit' or \^D when finished/)
 })
@@ -85,3 +85,3 @@
     t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
-    t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
+    t.match(output, /Exploring \{CWD\}\/([^/]+)\nType 'exit' or \^D when finished/)
 
EOF
@@ -72,3 +72,3 @@
t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
t.match(output, /Exploring \{CWD\}\/([^/]+)\nType 'exit' or \^D when finished/)
})
@@ -85,3 +85,3 @@
t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
t.match(output, /Exploring \{CWD\}\/([^/]+)\nType 'exit' or \^D when finished/)

Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@reggi
Copy link
Contributor Author

reggi commented Feb 14, 2025

the core cli tests are passing but these aren't represented now in their own

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@reggi