📦 Update dependency sinon to v10

[renovate-bot]

This PR contains the following updates:

Package	Change	Age	Adoption	Passing	Confidence
sinon (source)	9.2.4 -> 10.0.0

How to resolve breaking changes

This PR may introduce breaking changes that require manual intervention. In such cases, you will need to check out this branch, fix the cause of the breakage, and commit the fix to ensure a green CI build. To check out and update this PR, follow the steps below:

# Check out the PR branch (these steps are from GitHub)
git checkout -b renovate-bot-renovate/core-sinon-10.x main
git pull https://github.com/renovate-bot/amphtml.git renovate/core-sinon-10.x

# Directly make fixes and commit them
amp lint --fix # For lint errors in JS files
amp prettify --fix # For prettier errors in non-JS files
# Edit source code in case of new compiler warnings / errors

# Push the changes to the branch
git push git@github.com:renovate-bot/amphtml.git renovate-bot-renovate/core-sinon-10.x:renovate/core-sinon-10.x

---

Release Notes

sinonjs/sinon

v10.0.0

Compare Source

==================

• Upgrade nise to 4.1.0
• Use @​sinonjs/eslint-config@​4 => Adopts ES2017 => Drops support for IE 11, Legacy Edge and legacy Safari

---

Configuration

📅 Schedule: "after 12am every weekday" in timezone America/Los_Angeles.

🚦 Automerge: Enabled.

♻️ Rebasing: Renovate will not automatically rebase this PR, because other commits have been found.

🔕 Ignore: Close this PR and you won't be reminded about this update again.

---

• If you want to rebase/retry this PR, check this box.

---

This PR has been generated by WhiteSource Renovate. View repository job log here.

[erwinmombay]

@rsimha do we use node lts too for windows? it seems to be throwing a error on assert.fail(`${method} is not a function`); on IE11 (for the template literal starting back tick)

[rsimha]

do we use node lts too for windows?

Yes we do. See the Install dependencies section in the logs.

it seems to be throwing a error on assert.fail(`${method} is not a function`); on IE11 (for the template literal starting back tick)

This appears to be by design. There's a clue in the release notes and in the fact that the major version is changing. Looks like we need to run a babel transform against the module.

[rsimha]

Since sinon v10 has dropped support for IE 11, we're seeing this error.

I took a stab at getting esbuild to transform the contents of node_modules/sinon/ along with our test code, but hit an OOM error. See logs with additional logging:

@jridgewell Any idea what's going on here? Do we still care enough about running a small handful of tests on IE 11? Perhaps it's time to drop IE support?

[jridgewell]

Inspecting the output, I see on line 83 of the file run by IE11:

                    assert.fail(`${method} is not a function`);

This means we're either not transforming the file with Babel, or esbuild is outputting ES6 syntax instead of ES5.

[jridgewell]

esbuild is set to es5:

amphtml/build-system/tasks/runtime-test/runtime-test-base.js

Line 278 in 4f69d04

target: 'es5',

, so we're not Babel compiling the file. Likely because of a node_modules exclusion?

[rsimha]

so we're not Babel compiling the file. Likely because of a node_modules exclusion?

Correct, we weren't transforming node_modules/sinon/ earlier. When I tried doing so, esbuild threw an OOM error. See the second screenshot in #33418 (comment).

[jridgewell]

What change did you make?

[rsimha]

What change did you make?

diff --git a/build-system/test-configs/config.js b/build-system/test-configs/config.js
index e2ea67e47..3ca9457fc 100644
--- a/build-system/test-configs/config.js
+++ b/build-system/test-configs/config.js
@@ -82,6 +82,7 @@ const karmaJsPaths = [
   'ads/**/test/test-*.js',
   'extensions/**/test/**/*.js',
   'testing/**/*.js',
+  'node_modules/sinon/**/.js',
 ];

Another thing I just noticed is that this change is introducing a silent failure on other platforms as well. For example, here. I think we should just throw an error when zero tests are detected (unlike earlier, when zero tests was a mostly harmless transitional condition). Edit: Sent out #34040 with this change.

[amp-owners-bot]

Hey @rsimha! These files were changed:

build-system/common/esbuild-babel.js

[rsimha]

Bravo, @jridgewell! (And thanks for digging into the root cause.)