nodejs
lookup: add babel
Work in progress because babel is still not compatible with citgm. This is what you have to do now to test it:
make bootstrap
make test
Work in progress because babel is still not compatible with citgm. This is what you have to do now to test it:
make bootstrap make test
Does babel have any equivalent package.json scripts for the make targets? If so we could use https://github.com/nodejs/citgm/pull/696.
@richardlau yes: https://github.com/babel/babel/blob/165ef2941694184d83307b41d311751bd3dabae0/package.json#L5-L10
Actually no, the bootstrap script is missing from package.json. We could open a PR to add it. Currently this works (and tests all pass with 11.13.0) if yarn is installed:
make bootstrap
npm run test
Actually no, the
bootstrapscript is missing frompackage.json. We could open a PR to add it.
Maybe that would be simplest and would work with https://github.com/nodejs/citgm/pull/696 in its current form.
Alternative options include expanding https://github.com/nodejs/citgm/pull/696 so that instead of being limited to package.json script we allow arbitrary commands (it would make running an alternative package.json script more verbose in the lookup), or we could add yet another option (postinstall?) that could take an arbritrary command to be run after install but before test.
Codecov Report
Merging #628 (3908bf1) into main (d0185d9) will not change coverage. The diff coverage is
n/a.
@@ Coverage Diff @@
## main #628 +/- ##
=======================================
Coverage 96.12% 96.12%
=======================================
Files 31 31
Lines 929 929
=======================================
Hits 893 893
Misses 36 36
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update d0185d9...3c2df9b. Read the comment docs.
It seems like we cannot make it work without babel being a git repository. /cc @nicolo-ribaudo
https://github.com/targos/citgm/runs/2046109107?check_suite_focus=true
error: | ➤ YN0000: │ husky@npm:3.1.0 STDOUT Command failed: git rev-parse --show-toplevel --git-common-dir
error: | ➤ YN0000: │ husky@npm:3.1.0 STDOUT fatal: not a git repository (or any of the parent directories): .git
error: | ➤ YN0000: │ husky@npm:3.1.0 STDOUT husky > Failed to install
error: | ::endgroup::
error: | ➤ YN0000: └ Completed in 29s 121ms
error: | ➤ YN0000: Done with warnings in 2m 9s
error: | rm -f tsconfig.json
error: | git clean packages/*/tsconfig.json -xfq
error: | Makefile:113: recipe for target 'clean-tsconfig' failed
error: |
error: | fatal: not a git repository (or any of the parent directories): .git
error: | make: *** [clean-tsconfig] Error 128
I can fix it in our scripts. ~~Do you know if there is any other project in citgm that depends on husky for commit hooks?~~
I think the main issue is the git clean command. Husky fails but not in a critical way
Actually, it seems yarn install && yarn build && yarn test works because it doesn't cat that git command.
bootstrap is install + clean + build and clean is the step that uses git, but it's not needed with a fresh clone of the repo.
New run with the install build test sequence: https://github.com/targos/citgm/actions/runs/627579454
We test Babel on Node.js 10 so it should work without problems (there are a few disabled tests that rely on native ESM). However, we'll drop Node.js 10 support in a few months.
Well, it failed on Node.js 10 because of ESM tests. I don't know why they aren't properly disabled but I think we can safely skip Node.js 10. It's in maintenance and the few patches that we will still do to it (if any) are probably not going to break Babel.
CI on all platforms: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker-pipeline/140/console
New CI: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker-pipeline/141/
@nodejs/citgm I think this is ready to land. There were two errors in the last CI run: https://ci.nodejs.org/job/citgm-smoker-nobuild/1050/nodes=win10-vs2019/console https://ci.nodejs.org/job/citgm-smoker-nobuild/1050/nodes=aix71-ppc64/console
But they are not related to babel, as they happen before citgm is even downloaded.
Hey all, I made a boo boo on main and had to force push. I've rebased this branch and force pushed to make sure that you don't have to do extra work because of my mistake
New CI: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker-pipeline/149/
Codecov Report
All modified lines are covered by tests :white_check_mark:
Comparison is base (
460c3a0) 96.46% compared to head (01aedc2) 96.44%. Report is 32 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #628 +/- ##
==========================================
- Coverage 96.46% 96.44% -0.02%
==========================================
Files 28 28
Lines 2149 2139 -10
==========================================
- Hits 2073 2063 -10
Misses 76 76
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
The failing tests:
describe("deprecated types and aliases", () => {
beforeAll(() => {
jest.spyOn(console, "warn").mockImplementation(() => {});
});
afterEach(() => {
console.warn.mockClear();
});
afterAll(() => {
console.warn.mockRestore();
});
it("should warn for deprecated node types", function testFn1() {
const visitNumericLiteral = () => {};
const visitor = {
NumberLiteral: visitNumericLiteral,
};
traverse.explode(visitor);
expect(console.warn).toHaveBeenCalledWith(
expect.stringMatching(
/Visitor `NumberLiteral` has been deprecated, please migrate to `NumericLiteral`[^]+at.+testFn1/,
),
);
expect(visitor).toHaveProperty("NumericLiteral.enter", [
visitNumericLiteral,
]);
});
it("should warn for deprecated aliases", function testFn2() {
const visitImportOrExportDeclaration = () => {};
const visitor = {
ModuleDeclaration: visitImportOrExportDeclaration,
};
traverse.explode(visitor);
expect(console.warn).toHaveBeenCalledWith(
expect.stringMatching(
/Visitor `ModuleDeclaration` has been deprecated, please migrate to `ImportOrExportDeclaration`[^]+at.+testFn2/,
),
);
expect(visitor).toHaveProperty("ImportDeclaration.enter", [
visitImportOrExportDeclaration,
]);
});
it("should not warn deprecations if usage comes from a @babel/* package", () => {
const visitImportOrExportDeclaration = () => {};
const visitor = {
ModuleDeclaration: visitImportOrExportDeclaration,
};
callFromAtBabelPackage(traverse.explode, visitor);
expect(console.warn).not.toHaveBeenCalled();
expect(visitor).toHaveProperty("ImportDeclaration.enter", [
visitImportOrExportDeclaration,
]);
});
});
It looks like jest.spyOn(console, "warn") does not work in citgm. If it's expected, and it there is a way to detect that we are running in citgm, I could just skip those tests in Babel.
Actually, this is the (related) error I get running gitgm locally for Babel:
error: | [12:14:10] TypeError: Cannot redefine property: __internal__deprecationWarning
error: | at Function.defineProperty (<anonymous>)
error: | at Object.<anonymous> (/private/var/folders/bg/d5r9tspx5hn87zp52mcl82dh0000gn/T/bcde
error: | at Module._compile (node:internal/modules/cjs/loader:1241:14)
error: | at Module._extensions..js (node:internal/modules/cjs/loader:1295:10)
error: | at Module.load (node:internal/modules/cjs/loader:1091:32)
error: | at Module._load (node:internal/modules/cjs/loader:938:12)
error: | at Module.require (node:internal/modules/cjs/loader:1115:19)
error: | at require (node:internal/modules/helpers:130:18)
error: | at Object.<anonymous> (/private/var/folders/bg/d5r9tspx5hn87zp52mcl82dh0000gn/T/bcde
error: | at Module._compile (node:internal/modules/cjs/loader:1241:14)
Maybe it's just a bug on our side.
EDIT: This is the Node.js 20.6 regression, let me try with an older one 🙃
The other failure (the one that logs EXECUTOR TIMEOUT) is a test that's flaky when I run them locally, but I never saw it being flaky on GitHub actions. I'll mark it as optional.
There are two failures:
- The one that logs
EXECUTOR TIMEOUTis a test that's flaky when I run them locally, but I never saw it being flaky on GitHub actions. I'll mark it as optional, so that it can be skipped in citgm (https://github.com/babel/babel/pull/15928) - The one about the missing deprecation warning is because apparently that test does not support running when the monorepo is nested inside a folder named
@babel/, due to the check we perform here. Would it be possible to not download the repository in<tmp dir>/@babel/core/, but in something like<tmp dir>/babel?@babel/coreis not the monorepo itself, it's one of the packages. If not, we can just skip that test when we detect this condition.
@nicolo-ribaudo
Can you try this? It doesn't work locally on me. :)
"@babel/core": {
"maintainers": "nicolo-ribaudo",
"useGitClone": true,
"head": true,
"yarn": true,
"repo": "https://github.com/babel/babel",
"scripts": ["bootstrap", "test"],
"skip": ["aix", "s390"]
},
Edited: I found the cause of the error: Filename too long. Trying.
@targos Could you re-trigger the test run? On the latest release tag all tests should be passing (except for Node.js 20 obviously)
Hey, could somebody re-trigger a test run? Node.js has fixed the bug that was making the Babel tests fail, so it should be green now.