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.

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

[Test]: Jest to Vitest migration caravan-psbt package #133

Merged
merged 16 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
767 changes: 660 additions & 107 deletions package-lock.json

Large diffs are not rendered by default.

14 changes: 0 additions & 14 deletions packages/caravan-psbt/jest.config.ts

This file was deleted.

1 change: 0 additions & 1 deletion packages/caravan-psbt/jest.setup.ts

This file was deleted.

15 changes: 6 additions & 9 deletions packages/caravan-psbt/package.json
bucko13 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@
"scripts": {
"build": "tsup src/index.ts --format cjs,esm --dts",
"dev": "npm run build -- --watch",
"test": "NODE_OPTIONS=--experimental-vm-modules npx jest src",
"lint": "eslint src",
"ci": "npm run lint && npm run test",
"test:watch": "npm run test -- --watch",
"test:debug": "node --inspect-brk ../../node_modules/.bin/jest --runInBand"
"test": "vitest --watch=false src",
"test:watch": "vitest --watch src",
"test:debug": "node --inspect-brk vitest --runInBand"
},
"keywords": [
"bitcoin",
Expand All @@ -54,18 +54,15 @@
"@caravan/eslint-config": "*",
"@caravan/multisig": "*",
"@caravan/typescript-config": "*",
"@inrupt/jest-jsdom-polyfills": "^3.2.1",
"@jest/globals": "^29.7.0",
"@types/jest": "^29.5.12",
"@vitejs/plugin-react": "^4.3.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this necessary?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had moved that dependency to the root package.json file as of which I've removed it from here

"eslint-plugin-prettier": "^5.1.3",
"jest": "^29.4.1",
"jsdom": "24.0.0",
"jsdom-global": "3.0.2",
"lodash": "^4.17.21",
"react-silence": "^1.0.4",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you replaced the uses of this right? so can probably uninstall.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I haven't uninstalled them completely since those modules are shared in other packages, if uninstalled the test suite there will fail.

I'll remove those unwanted modules once every package gets migrated to vitest : )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh. That shouldn’t be the case with the monorepo since they should be isolated. Any package that uses this should have it in its own package.json

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, every package has its own package.json but the node_modules and package-lock.json files are shared among them

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right so you should be able to remove it from THIS package.json, re-run npm install to update the package lock accordingly and then everything else should still worn

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've re-run the npm i command and pushed the changes in package-lock.json file in my latest commit. I've also checked if the tests are running fine and they are.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right I don’t imagine anything has changed. It looks like react-silence is still in this packages package.json. I think you can remove it and then re-run everything and it should be fine. If it’s not working then that means another package needs to be explicit about the react silence dev dependency.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed react-silence from package.json of psbt but after doing so the tests in caravan-bitcoin were failing so I've installed the same in its own package.json file

"ts-jest": "^29.1.2",
"ts-node": "^10.9.2",
"tsup": "^7.2.0",
"typescript": "^5.2.2"
"typescript": "^5.2.2",
"vitest": "^2.0.5"
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { bufferize } from "./functions";
import { bufferize } from "../functions";
import { describe, it, expect } from "vitest";

DhairyaMajmudar marked this conversation as resolved.
Show resolved Hide resolved
describe("bufferize", () => {
it("should return a buffer when given a hex string", () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
/**
* @jest-environment jsdom
*/

import {
generateMultisigFromHex,
P2WSH,
Expand All @@ -12,9 +8,11 @@ import {
getUnsignedMultisigPsbtV0,
validateMultisigPsbtSignature,
translatePSBT,
} from "./psbt";
} from "../../psbtv0/psbt";
import _ from "lodash";
import { psbtArgsFromFixture } from "./utils";
import { psbtArgsFromFixture } from "../../psbtv0/utils";

import { describe, expect, it, test } from "vitest";

describe("getUnsignedMultisigPsbtV0", () => {
TEST_FIXTURES.transactions
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { autoLoadPSBT } from "./utils";
import { autoLoadPSBT } from "../../psbtv0/utils";
import { describe, expect, it } from "vitest";

describe("autoLoadPSBT", () => {
it("should fail if you don't send a String", () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
import { PsbtV2, getPsbtVersionNumber } from "./";
import { test, jest } from "@jest/globals";
import { silenceDescribe } from "react-silence";

import { KeyType, PsbtGlobalTxModifiableBits } from "./types";
import { PsbtV2, getPsbtVersionNumber } from "../../psbtv2";
import {
vi,
describe,
it,
expect,
beforeEach,
test,
beforeAll,
afterAll,
} from "vitest";

import { KeyType, PsbtGlobalTxModifiableBits } from "../../psbtv2/types";

const BIP_370_VECTORS_INVALID_PSBT = [
// Case: PSBTv0 but with PSBT_GLOBAL_VERSION set to 2.
Expand Down Expand Up @@ -723,7 +731,14 @@ const BIP_174_VECTORS_VALID_PSBT = [
];

describe("PsbtV2", () => {
silenceDescribe("error", "warn");
beforeAll(() => {
vi.spyOn(console, "error").mockImplementation(() => {});
vi.spyOn(console, "warn").mockImplementation(() => {});
});

afterAll(() => {
vi.restoreAllMocks();
});

test.each(BIP_370_VECTORS_INVALID_PSBT)(
"Throws with BIP0370 test vectors. $case",
Expand Down Expand Up @@ -895,9 +910,9 @@ describe("PsbtV2.isReadyForSigner", () => {
});

it("Returns not ready for Signer when the psbt has already finalized inputs", () => {
jest
.spyOn(psbt, "isReadyForTransactionExtractor", "get")
.mockReturnValue(true);
vi.spyOn(psbt, "isReadyForTransactionExtractor", "get").mockReturnValue(
true,
);
expect(psbt.isReadyForSigner).toBe(false);
});

Expand Down Expand Up @@ -1066,7 +1081,15 @@ describe("PsbtV2.nLockTime", () => {
});

describe("PsbtV2.FromV0", () => {
silenceDescribe("error", "warn");
beforeAll(() => {
vi.spyOn(console, "error").mockImplementation(() => {});
vi.spyOn(console, "warn").mockImplementation(() => {});
});

// Restore console methods
afterAll(() => {
vi.restoreAllMocks();
});

test.each(BIP_174_VECTORS_INVALID_PSBT)(
"Throws with BIP0174 test vectors. $case",
Expand Down Expand Up @@ -1161,13 +1184,20 @@ describe("getPsbtVersionNumber", () => {
});

describe("PsbtV2.addPartialSig", () => {
silenceDescribe("error", "warn");
beforeAll(() => {
vi.spyOn(console, "error").mockImplementation(() => {});
vi.spyOn(console, "warn").mockImplementation(() => {});
});

// Restore console methods
afterAll(() => {
vi.restoreAllMocks();
});
let psbt;

beforeEach(() => {
psbt = new PsbtV2();
psbt.handleSighashType = jest.fn();
psbt.handleSighashType = vi.fn();
// This has to be added so inputs can be added else addSig will fail since
// the input at the index does not exist.
psbt.PSBT_GLOBAL_TX_MODIFIABLE = ["INPUTS"];
Expand Down Expand Up @@ -1229,7 +1259,7 @@ describe("PsbtV2.removePartialSig", () => {

beforeEach(() => {
psbt = new PsbtV2();
psbt.handleSighashType = jest.fn();
psbt.handleSighashType = vi.fn();
// This has to be added so inputs can be added else removeSig will fail
// since the input at the index does not exist.
psbt.PSBT_GLOBAL_TX_MODIFIABLE = ["INPUTS"];
Expand Down
2 changes: 1 addition & 1 deletion packages/caravan-psbt/src/psbtv0/psbt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { GlobalXpub } from "bip174/src/lib/interfaces.js";
// be sorted out and simplified then we can use the primary module with wasm
import * as ecc from "../../vendor/tiny-secp256k1-asmjs/lib/index.js";
import * as bitcoin from "bitcoinjs-lib-v6";
import { bufferize } from "src/functions";
import { bufferize } from "../functions";
import BigNumber from "bignumber.js";
import { reverseBuffer } from "bitcoinjs-lib-v6/src/bufferutils.js";
import { autoLoadPSBT } from "./utils";
Expand Down
9 changes: 2 additions & 7 deletions packages/caravan-psbt/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,9 @@
"extends": "@caravan/typescript-config/base.json",
"compilerOptions": {
"baseUrl": ".",
"types": [
"node",
"jest"
],
"types": ["node", "vitest/globals"],
DhairyaMajmudar marked this conversation as resolved.
Show resolved Hide resolved
"paths": {
"src/*": [
"./src/*"
]
"src/*": ["./src/*"]
}
}
}
2 changes: 1 addition & 1 deletion vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ export default defineConfig({
test: {
include: ["./**/*.test.ts"],
globals: true,
environment: "jsdom",
environment: "node",
},
DhairyaMajmudar marked this conversation as resolved.
Show resolved Hide resolved
});
Loading