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.env config not working in workspace project #5016

Closed
6 tasks done
hi-ogawa opened this issue Jan 20, 2024 · 3 comments · Fixed by #5476
Closed
6 tasks done

test.env config not working in workspace project #5016

hi-ogawa opened this issue Jan 20, 2024 · 3 comments · Fixed by #5476
Labels
discussion feat: workspace Issues and PRs related to the workspace feature

Comments

@hi-ogawa
Copy link
Contributor

hi-ogawa commented Jan 20, 2024

Describe the bug

The issue is originally reported in the discussion #5003 (I initially suspected mis-usage of loadEnv with process.cwd, but it turned out to be irrelevant).

The reproduction is here https://stackblitz.com/edit/github-2srz39?file=packages%2Flib1%2Fvite.config.ts

Given following configurations:

//// vitest.workspace.ts
import { defineWorkspace } from 'vitest/config'

export default defineWorkspace([
  'packages/*',
])


//// packages/lib1/vite.config.ts
import { defineProject } from 'vitest/config';

export default defineProject({
  test: {
    env: {
      SOME_ENV: 'hello',   // <-- "env" is not applied per project
    },
  },
  define: {
    SOME_DEFINE: '"one"',  // <-- "define" is applied per project
  },
});

Running tests by npm -C packages/lib1 test succeeds but npm test fails.

One simple workaround I thought of is to use setupFiles and do this Object.assign(process.env, { ... your env ... }) manually.


It looks like currently only the root test.env config is eventually passed to new Tinypool constructor (and shared between all thread/fork instances) but per-project test.env config seems to be not used.

const options: PoolProcessOptions = {
execArgv: [
...execArgv,
...conditions,
],
env: {
TEST: 'true',
VITEST: 'true',
NODE_ENV: process.env.NODE_ENV || 'test',
VITEST_MODE: ctx.config.watch ? 'WATCH' : 'RUN',
...process.env,
...ctx.config.env,
},
}


Also btw, I noticed env config is not currently documented in https://vitest.dev/config/ but I also somehow knew this config (and actually recommended in remix vite doc https://remix.run/docs/en/main/future/vite#plugin-usage-with-other-vite-based-tools-eg-vitest-storybook). At least this is available from defineProject typing, so I suppose this feature is meant to work for project level as well.

Reproduction

https://stackblitz.com/edit/github-2srz39?file=packages%2Flib1%2Fvite.config.ts

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.18.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.14.0 - /usr/local/bin/pnpm
  npmPackages:
    @vitest/ui: latest => 1.2.1 
    vite: latest => 5.0.11 
    vitest: latest => 1.2.1

Used Package Manager

npm

Validations

@hi-ogawa hi-ogawa added pending triage bug p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Jan 20, 2024
@sheremet-va
Copy link
Member

sheremet-va commented Jan 20, 2024

I don't think it's meant to work in projects. Our architecture doesn't allow for envs there (like you already mentioned, it's passed down once when tinypool is initialized)

@hi-ogawa
Copy link
Contributor Author

hi-ogawa commented Jan 20, 2024

I understand the current limitation, but I thought this feature would be nice-to-have for users if they simply expect process.env to setup around the same time as setupFiles instead of underlying fork or new Worker level. (which was also my initial expectation before reading the code).

Is it critical to allow overriding environment variable at the underlying level? For example, what I was thinking was to maybe we could do Object.assign(process.env, ctx.config.env) during the common worker entry or somewhere.

(I suppose NODE_OPTIONS would be only effective during fork/thread setup though I'm not sure what would be the use case not covered by execArgv)

@hi-ogawa hi-ogawa added pending triage and removed bug p3-minor-bug An edge case that only affects very specific usage (priority) labels Jan 20, 2024
@sheremet-va
Copy link
Member

If we do that, then we should use only a single way to assign env variables then.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discussion feat: workspace Issues and PRs related to the workspace feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants