Skip to content

Commit

Permalink
Remove sinon in favor of existing APIs of playwright and vitest
Browse files Browse the repository at this point in the history
We where using `sinon` in the `playwright` tests to simulate a different
time, which since `playwright@1.45.0` can be done with the Clock API
calling `page.clock.setFixedTime`.

And to test the mutex executor we can use the vitest utils spy function
`vi.fn()` instead of `sinon.spy()`
  • Loading branch information
sebastinez committed Aug 29, 2024
1 parent 7c9f1a0 commit a34056b
Show file tree
Hide file tree
Showing 14 changed files with 19 additions and 269 deletions.
128 changes: 0 additions & 128 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
"@types/lodash": "^4.17.7",
"@types/md5": "^2.3.5",
"@types/node": "^20.14.12",
"@types/sinon": "^17.0.3",
"@types/wait-on": "^5.3.4",
"@typescript-eslint/parser": "^8.3.0",
"chalk": "^5.3.0",
Expand All @@ -44,7 +43,6 @@
"happy-dom": "^15.0.0",
"prettier": "^3.3.3",
"prettier-plugin-svelte": "^3.2.6",
"sinon": "^18.0.0",
"svelte-check": "^3.8.6",
"svelte-eslint-parser": "^0.41.0",
"typescript": "^5.5.4",
Expand Down
9 changes: 1 addition & 8 deletions tests/e2e/repo/commit.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
test,
} from "@tests/support/fixtures.js";
import { changeBranch } from "@tests/support/repo";
import sinon from "sinon";

const commitUrl = `${sourceBrowsingUrl}/commits/${bobHead}`;

Expand All @@ -20,13 +19,7 @@ test("navigation from commit list", async ({ page }) => {
});

test("relative timestamps", async ({ page }) => {
await page.addInitScript(() => {
sinon.useFakeTimers({
now: new Date("December 21 2022 12:00:00").valueOf(),
shouldClearNativeTimers: true,
shouldAdvanceTime: false,
});
});
await page.clock.setFixedTime(new Date("December 21 2022 12:00:00"));
await page.goto(commitUrl);
await expect(
page.getByText(`Bob Belcher committed ${shortBobHead}`),
Expand Down
10 changes: 1 addition & 9 deletions tests/e2e/repo/commits.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
test,
} from "@tests/support/fixtures.js";
import { changeBranch, createRepo } from "@tests/support/repo";
import sinon from "sinon";

test("peer and branch switching", async ({ page }) => {
await page.goto(sourceBrowsingUrl);
Expand Down Expand Up @@ -149,14 +148,7 @@ test("expand commit message", async ({ page }) => {
});

test("relative timestamps", async ({ page }) => {
await page.addInitScript(() => {
sinon.useFakeTimers({
now: new Date("December 21 2022 12:00:00").valueOf(),
shouldClearNativeTimers: true,
shouldAdvanceTime: false,
});
});

await page.clock.setFixedTime(new Date("December 21 2022 12:00:00"));
await page.goto(sourceBrowsingUrl);
await page
.getByRole("link", { name: `Commits ${aliceMainCommitCount}` })
Expand Down
22 changes: 1 addition & 21 deletions tests/support/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type * as Stream from "node:stream";

import * as Fs from "node:fs/promises";
import * as Path from "node:path";
import { fileURLToPath } from "node:url";
import { test as base, expect } from "@playwright/test";
import { execa } from "execa";

Expand All @@ -32,14 +31,6 @@ export const test = base.extend<{
forAllTests: [
async ({ outputLog, page }, use) => {
const browserLabel = logLabel.logPrefix("browser");
let sinonPath = fileURLToPath(import.meta.resolve("sinon"));
// The exports in sinon-esm.js mess up our test pipeline
if (sinonPath.endsWith("-esm.js")) {
sinonPath = sinonPath.replace("-esm", "");
}
await page.addInitScript({
path: sinonPath,
});
page.on("console", msg => {
// Ignore common console logs that we don't care about.
if (
Expand All @@ -48,18 +39,7 @@ export const test = base.extend<{
msg.text().startsWith("Not able to parse url") ||
msg
.text()
.includes("Please make sure it wasn't preloaded for nothing.") ||
// @sinonjs/fake-timers uses a global variable called `timers` which
// is also used by node, so vite erronously detects this and shows a
// warning whenever we install fake timers in tests. We suppress the
// warning here to avoid clogging the logs. For more info see:
//
// https://vitejs.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility
msg
.text()
.startsWith(
'Module "timers" has been externalized for browser compatibility.',
)
.includes("Please make sure it wasn't preloaded for nothing.")
) {
return;
}
Expand Down
9 changes: 4 additions & 5 deletions tests/unit/mutexExecutor.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as sinon from "sinon";
import { describe, expect, test } from "vitest";
import { describe, expect, test, vi } from "vitest";

import * as mutexExecutor from "@app/lib/mutexExecutor";
import { sleep } from "@app/lib/sleep";
Expand Down Expand Up @@ -58,17 +57,17 @@ describe("executor", () => {

test("triggers abort signal event", async () => {
const e = mutexExecutor.create();
const abortListener = sinon.spy();
const abortListener = vi.fn();

void e.run(async abort => {
abort.addEventListener("abort", abortListener);
await sleep(10);
return "first";
});
expect(abortListener.called).toBe(false);
expect(abortListener).not.toHaveBeenCalled();
// eslint-disable-next-line @typescript-eslint/no-empty-function
void e.run(async () => {});
expect(abortListener.called).toBe(true);
expect(abortListener).toHaveBeenCalled();
});

test("don’t throw error on aborted task", async () => {
Expand Down
9 changes: 1 addition & 8 deletions tests/visual/desktop/cob.spec.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
import { test, expect, cobUrl } from "@tests/support/fixtures.js";
import sinon from "sinon";

test.beforeEach(async ({ page }) => {
await page.addInitScript(() => {
sinon.useFakeTimers({
now: new Date("November 24 2022 12:00:00").valueOf(),
shouldClearNativeTimers: true,
shouldAdvanceTime: false,
});
});
await page.clock.setFixedTime(new Date("November 21 2022 12:00:00"));
});

test("issues page", async ({ page }) => {
Expand Down
13 changes: 2 additions & 11 deletions tests/visual/desktop/landingPage.spec.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,14 @@
import { test, expect } from "@tests/support/fixtures.js";
import sinon from "sinon";

test("pinned repos", async ({ page }) => {
await page.addInitScript(() => {
localStorage.setItem(
"configuredPreferredSeeds",
JSON.stringify([{ hostname: "127.0.0.1", port: 8081, scheme: "http" }]),
);
sinon.useFakeTimers({
now: new Date("November 24 2022 12:00:00").valueOf(),
shouldClearNativeTimers: true,
shouldAdvanceTime: false,
});
});

await page.clock.setFixedTime(new Date("November 24 2022 12:00:00"));
await page.goto("/", { waitUntil: "networkidle" });
await expect(page).toHaveScreenshot();
});
Expand All @@ -24,13 +19,9 @@ test("load repos error", async ({ page }) => {
"configuredPreferredSeeds",
JSON.stringify([{ hostname: "127.0.0.1", port: 8081, scheme: "http" }]),
);
sinon.useFakeTimers({
now: new Date("November 24 2022 12:00:00").valueOf(),
shouldClearNativeTimers: true,
shouldAdvanceTime: false,
});
});

await page.clock.setFixedTime(new Date("November 24 2022 12:00:00"));
await page.route(
({ pathname }) => pathname === "/api/v1/repos",
route => route.fulfill({ status: 500 }),
Expand Down
Loading

0 comments on commit a34056b

Please sign in to comment.