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

Feature Request: specify os in options.json #10252

Closed
JLHwung opened this issue Jul 22, 2019 · 2 comments · Fixed by #10249
Closed

Feature Request: specify os in options.json #10252

JLHwung opened this issue Jul 22, 2019 · 2 comments · Fixed by #10249
Labels
area: tests i: enhancement outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@JLHwung
Copy link
Contributor

JLHwung commented Jul 22, 2019

Feature Request

Is your feature request related to a problem? Please describe.
yarn jest is failing on Windows. All the failures are related to the path separator. For example

- var _jsxFileName = "/fake/path/mock.js";
+ var _jsxFileName = "C:\\fake\\path\\mock.js";
FAIL packages/babel-preset-react/test/preset-options.js (5.472s)
  ● babel-preset-react/preset options › development
C:\babel\packages\babel-preset-react\test/fixtures/preset-options/development/exec.js: expect(received).toBe(expected) // Object.is equality

- Expected
+ Received

@@ -1,6 +1,6 @@
- var _jsxFileName = "/fake/path/mock.js";
+ var _jsxFileName = "C:\\fake\\path\\mock.js";
  React.createElement(Foo, {
    bar: "baz",
    __source: {
      fileName: _jsxFileName,
      lineNumber: 1
  1 | const actual = transform('<Foo bar="baz" />', Object.assign({}, opts, {
  2 |   filename: '/fake/path/mock.js'
  3 | })).code;
  4 | const expected = multiline(['var _jsxFileName = "/fake/path/mock.js";', 'React.createElement(Foo, {', '  bar: "baz",', '  __source: {', '    fileName: _jsxFileName,', '    lineNumber: 1', '  },', '  __self: this', '});']);
  5 | expect(actual).toBe(expected);

  3 |   Object.assign({}, opts, { filename: '/fake/path/mock.js' })
  4 | ).code;
> 5 | 
    | ^
  6 | const expected = multiline([
  7 |   'var _jsxFileName = "/fake/path/mock.js";',
  8 |   'React.createElement(Foo, {',

  at packages/babel-preset-react/test/fixtures/preset-options/development/exec.js:5:16
  at runCodeInTestContext (packages/babel-helper-transform-fixture-test-runner/lib/index.js:238:7)
  at run (packages/babel-helper-transform-fixture-test-runner/lib/index.js:305:20)
  at Object.<anonymous> (packages/babel-helper-transform-fixture-test-runner/lib/index.js:430:30)

When we are injecting a file path to the source code, the behavior will be os-dependent. It would be convenient if we can specify os in options.json so that text-runner will run different subset of test cases. The idea comes from #10242 (comment)

Describe the solution you'd like
Specify an os field in options.json,

{
  "os": Array<"windows" | "macos" | "linux" | "freebsd" | "openbsd" | "sunos" | "aix">
}

If os is not specified, it means the test case should be run on every platform we supported.

Example: a test cases specifies

{
  "os": ["macos", "linux"]
}

will only run on Linux and macOS machines.

OS Detection

Map process.platform to normalized name when necessary:

{
  win32: "windows",
  darwin: "macos",
}

Describe alternatives you've considered
Since all of cross-platform test failure comes from path separator, we may also instead define os be of a simplified subset of process.platform

{
  "os": Array<"windows" | "posix">
}

where "posix" = "macos" | "linux" | "freebsd" | "openbsd" | "sunos" | "aix".

Personally I prefer the second approach and we could easily extend our os definitions when we come across other os-dependent issues.

Teachability, Documentation, Adoption, Migration Strategy
This option is for internal use only. It should have no impact on our users.

Previous art

minNodeVersion: #5765.

Related

This feature will work with #10249 to enable windows building on travis.

@babel-bot
Copy link
Collaborator

Hey @JLHwung! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite.

@JLHwung
Copy link
Contributor Author

JLHwung commented Sep 27, 2019

Fixed by #10249

@JLHwung JLHwung closed this as completed Sep 27, 2019
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Dec 27, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: tests i: enhancement outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants