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

Environment options are not parsed from comments #1962

Closed
6 tasks done
nickmccurdy opened this issue Sep 3, 2022 · 8 comments
Closed
6 tasks done

Environment options are not parsed from comments #1962

nickmccurdy opened this issue Sep 3, 2022 · 8 comments
Labels
enhancement New feature or request

Comments

@nickmccurdy
Copy link
Contributor

nickmccurdy commented Sep 3, 2022

Describe the bug

Vitest supports parsing @{vitest,jest}-environment comments to set an environment, but it should also support @{vitest,jest}-environment-options comments for compatibility with Jest.

Reproduction

index.test.js

/**
 * @jest-environment jsdom
 * @jest-environment-options {"url": "https://example.com/"}
 */
test("use jsdom and set the URL in this test file", () => {
  expect(location.href).toBe("https://example.com/")
})

vite.config.js

export default {
  test: {
    globals: true,
  },
}

Expected result from Jest

 PASS  ./index.test.js
  ✓ use jsdom and set the URL in this test file (2 ms)

Actual result from Vitest

 FAIL  index.test.js > use jsdom and set the URL in this test file
AssertionError: expected 'http://localhost:3000/' to be 'https://example.com/' // Object.is equality
 ❯ index.test.js:6:25
      4|  */
      5| test("use jsdom and set the URL in this test file", () => {
      6|   expect(location.href).toBe("https://example.com/")
       |                         ^
      7| })
      8| 

  - Expected   "https://example.com/"
  + Received   "http://localhost:3000/"

System Info

System:
    OS: macOS 12.5.1
    CPU: (4) x64 Intel(R) Core(TM) i5-5257U CPU @ 2.70GHz
    Memory: 43.91 MB / 8.00 GB
    Shell: 3.5.1 - /usr/local/bin/fish
  Binaries:
    Node: 18.8.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 8.19.1 - /usr/local/bin/npm
    Watchman: 2022.08.29.00 - /usr/local/bin/watchman
  npmPackages:
    vitest: ^0.18.0 => 0.18.1

Used Package Manager

npm

Validations

@sheremet-va sheremet-va added the enhancement New feature or request label Sep 3, 2022
@nickmccurdy
Copy link
Contributor Author

nickmccurdy commented Sep 3, 2022

I just noticed that the JSDOM options are scoped under a jsdom property in setup, so there would have to be a change or adapter for that API to fix this.

@sheremet-va
Copy link
Member

sheremet-va commented Sep 3, 2022

I just noticed that the JSDOM options are scoped under a jsdom property in setup, so there would have to be a change or adapter for that API to fix this.

We can just put options inside a key of current env:

/**
 * @jest-environment jsdom
 * @jest-environment-options {"url": "https://example.com/"}
 */
// environmentOptions -> { jsdom: { url: string } }
/**
 * @jest-environment-options {"url": "https://example.com/"}
 */
// node is default environment. if config is used, will put into the one specified with config
// environmentOptions -> { node: { url: string } }

All spec files have environment, so should not be a problem.

@nickmccurdy
Copy link
Contributor Author

I'm mostly thinking about Jest compatibility, and it doesn't use a jsdom property. Can you pass top level options to an environment?

@sheremet-va
Copy link
Member

sheremet-va commented Sep 3, 2022

I'm mostly thinking about Jest compatibility, and it doesn't use a jsdom property.

environment will never be jest compatible, so I don't see any reason to think about jest compatibility here.

@nickmccurdy
Copy link
Contributor Author

nickmccurdy commented Sep 4, 2022

The environment API may not be compatible, but we already document compatibility with the @jest-environment comment, so I would prefer to to find a way where we can also be compatible with using @jest-environment-options with JSDOM without an intermediate property.

@sheremet-va
Copy link
Member

The environment API may not be compatible, but we already document compatibility with the @jest-environment comment, so I would prefer to to find a way where we can also be compatible with using @jest-environment-options with JSDOM without an intermediate property.

You wouldn't need to write @jest-environment-options { jsdom: { option } }, you would write @jest-environment-options { option }, since we already know the environment for the file.

@nickmccurdy
Copy link
Contributor Author

nickmccurdy commented Sep 4, 2022

Oh sorry, I think I misunderstood the purpose of the key. Is it just there so options other than those in the comments can be passed to the environment? Also can I still use that with a custom environment without having a 2D object?

@sheremet-va
Copy link
Member

Oh sorry, I think I misunderstood the purpose of the key. Is it just there so options other than those in the comments can be passed to the environment?

Yes, you can have multiple environments, so you need a key for each.

Also can I still use that with a custom environment without having a 2D object?

You would use it like this:

// config
export default {
  test: {
    environmentOptions: {
       custom: {} // your options
    }
  }
}

// or inside a spec file
/**
 * @vitest-environment custom
 * @vitest-environment-options { url: "http://example.com" } 
 */ 

nickmccurdy added a commit to nickmccurdy/vitest that referenced this issue Sep 6, 2022
nickmccurdy added a commit to nickmccurdy/vitest that referenced this issue Sep 6, 2022
nickmccurdy added a commit to nickmccurdy/vitest that referenced this issue Sep 6, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants