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

How to get CI tests running on MacOS? #687

Closed
justingrant opened this issue Jun 18, 2020 · 7 comments · Fixed by #689
Closed

How to get CI tests running on MacOS? #687

justingrant opened this issue Jun 18, 2020 · 7 comments · Fixed by #689

Comments

@justingrant
Copy link
Collaborator

I've been trying to get CI tests running on my Mac so that I can run npm test on the root Temporal package. I managed to unblock a few steps, but I'm stuck on Test262 throwing this exception below. Does this look familiar? I have Node 14 installed if that's relevant. Otherwise it's a pretty plain-vanilla MacOS environment.

FAIL Temporal/prop-desc.js (strict mode)
evalmachine.<anonymous>:8457
        const OFFSET = new RegExp(`^${REGEX.offset.source}$`);
                                      ^

ReferenceError: REGEX is not defined
    at evalmachine.<anonymous>:8457:32
    at evalmachine.<anonymous>:12694:2
    at Script.runInContext (vm.js:141:18)
    at Object.runInContext (vm.js:279:6)
    at Object.vm.runInESHostContext (/private/var/folders/5q/3x548yhd5cn1kpm9rq30qvbw0000gn/T/LhB5R89iRk7j1iDqyIH1/f-1592512064139-29759-1vla97e.1m7fh.js:13:19)
    at Object.<anonymous> (/private/var/folders/5q/3x548yhd5cn1kpm9rq30qvbw0000gn/T/LhB5R89iRk7j1iDqyIH1/f-1592512064139-29759-1vla97e.1m7fh.js:15:10)
    at Module._compile (internal/modules/cjs/loader.js:1200:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1220:10)
    at Module.load (internal/modules/cjs/loader.js:1049:32)
    at Function.Module._load (internal/modules/cjs/loader.js:937:14)

BTW, here's the commands that I needed to run to get it to run. Leaving this here as bread crumbs for the next victim.

  1. python3 -m pip install --upgrade pip
  2. python3 -m pip install virtualenv
  3. pip install ijson
  4. nproc doesn't exist in MacOS, so I changed this code inside ci_test.sh:
if [ "$(uname)" = 'Darwin' ]; then
  threads=$(sysctl -n hw.logicalcpu)
else
  threads=$(nproc --ignore 1)
fi
  1. Updated all deps of polyfill
@ptomato
Copy link
Collaborator

ptomato commented Jun 18, 2020

This seems like it might be a problem with rollup? Although I don't know what would make it platform-dependent.

These are probably the lines that it's referring to: https://github.com/tc39/proposal-temporal/blob/main/polyfill/lib/timezone.mjs#L20-L21
You might be able to take a look in the generated script.js that's prepended to each test262 test and see what's not getting bundled correctly.

@justingrant
Copy link
Collaborator Author

One weird thing is that ecmascript.mjs imports regex.mjs twice, with different names:

import * as PARSE from './regex.mjs';

import * as REGEX from './regex.mjs';

Is there any reason for this duplication, or is this just a bug?

BTW, I replaced one of these with an alias and this seemed to fix the problem above. Now I see this error below, repeated 100s of times, all with the same complaint about node internals being undefined.

FAIL Temporal/prop-desc.js (default)
internal/fs/utils.js:657
    throw new ERR_INVALID_ARG_TYPE(
    ^

TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received undefined
    at Object.writeFileSync (fs.js:1380:5)
    at evalmachine.<anonymous>:12703:4
    at Script.runInContext (vm.js:141:18)
    at Object.runInContext (vm.js:279:6)
    at Object.vm.runInESHostContext (/private/var/folders/5q/3x548yhd5cn1kpm9rq30qvbw0000gn/T/OTeKTZm1OjtupjTmxR3T/f-1592529148952-95840-t0r2vb.audi.js:13:19)
    at Object.<anonymous> (/private/var/folders/5q/3x548yhd5cn1kpm9rq30qvbw0000gn/T/OTeKTZm1OjtupjTmxR3T/f-1592529148952-95840-t0r2vb.audi.js:15:10)
    at Module._compile (internal/modules/cjs/loader.js:1200:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1220:10)
    at Module.load (internal/modules/cjs/loader.js:1049:32)
    at Function.Module._load (internal/modules/cjs/loader.js:937:14) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Looks like this may be related to Node setup. I'll try to reinstall Node and see if it helps.
nodejs/node#9355

@ptomato
Copy link
Collaborator

ptomato commented Jun 19, 2020

Is there any reason for this duplication, or is this just a bug?

Yes, I'm pretty sure this is a bug.

As for the other error — I'm pretty sure that evalmachine.<anonymous> indicates the script that test262 is testing, and I'm pretty sure the only usage of fs.writeFileSync() is here, so it seems that JSON.stringify(globalThis.__coverage__) might be returning undefined? This coda that gets tacked on to the test works fine for me when code coverage is turned off, but maybe it would be good to disable it when code coverage is inactive anyway.

@justingrant
Copy link
Collaborator Author

Yep that was it. globalThis.__coverage__ is undefined for me. Adding || {} fixed the problem for me.

fs.writeFileSync(filename, JSON.stringify(globalThis.__coverage__ || {}), 'utf-8');

When I get a chance, I'll send a PR for this and for the double-import in ecmascript.mjs.

@justingrant
Copy link
Collaborator Author

Hmm, while backing out all the other changes I tried that didn't help, I found one more change required to get test262 working in my environment. The problem is ReferenceError: Cannot access 'PARSE' before initialization here:

const OFFSET = new RegExp(`^${PARSE.offset.source}$`);

I tried two possible fixes. Both worked. Got a preference for which one I should to include in a "fix MacOS tests" PR?

  1. Move the initialization of OFFSET from module-global scope to function scope in the single function (parseOffsetString) that uses it.
  2. Change regex.mjs to use a global export.

Here's the full failure:

Expected no error, got ReferenceError: Cannot access 'PARSE' before initialization
FAIL Absolute/prototype/prop-desc.js (default)
evalmachine.<anonymous>:8457
        const OFFSET = new RegExp(`^${PARSE.offset.source}$`);
                                      ^

ReferenceError: Cannot access 'PARSE' before initialization
    at evalmachine.<anonymous>:8457:32
    at evalmachine.<anonymous>:12675:2
    at Script.runInContext (vm.js:141:18)
    at Object.runInContext (vm.js:279:6)
    at Object.vm.runInESHostContext (/private/var/folders/5q/3x548yhd5cn1kpm9rq30qvbw0000gn/T/eyvUng8YfOfLV0il7tUC/f-1592595224190-15025-19iy0w4.fkcg.js:13:19)
    at Object.<anonymous> (/private/var/folders/5q/3x548yhd5cn1kpm9rq30qvbw0000gn/T/eyvUng8YfOfLV0il7tUC/f-1592595224190-15025-19iy0w4.fkcg.js:15:10)
    at Module._compile (internal/modules/cjs/loader.js:1200:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1220:10)
    at Module.load (internal/modules/cjs/loader.js:1049:32)
    at Function.Module._load (internal/modules/cjs/loader.js:937:14)

@ptomato
Copy link
Collaborator

ptomato commented Jun 19, 2020

It seems like (1) would be simpler.

@justingrant
Copy link
Collaborator Author

False alarm. Problem turned out to be a rollup bug introduced 2 days ago and fixed today.
https://github.com/rollup/rollup/releases/tag/v2.17.1

Before figuring that out, I ended up doing the work to upgrade and verify all build-time dependencies, so I'll PR a dep upgrade too while I'm in the neighborhood.

justingrant added a commit to justingrant/proposal-temporal that referenced this issue Jun 19, 2020
Fixes tc39#687 which prevented Test262 tests from running in my
MacOS/Node14 environment.
- if MacOS, use sysctl instead of nproc in ci_test.sh
- defend against undefined `globalThis.__coverage__`
ptomato pushed a commit that referenced this issue Jun 19, 2020
Fixes #687 which prevented Test262 tests from running in my
MacOS/Node14 environment.
- if MacOS, use sysctl instead of nproc in ci_test.sh
- defend against undefined `globalThis.__coverage__`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants