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

Add global add prefix test #2025

Merged
merged 3 commits into from Nov 27, 2016
Merged

Conversation

torifat
Copy link
Member

@torifat torifat commented Nov 24, 2016

Test case for yarn global add --prefix <path> <package> & PREFIX=<path> yarn global add <package>

@torifat
Copy link
Member Author

torifat commented Nov 24, 2016

This is weird 😞

I have added this for testing. Running yarn test works, jest global works. But, npm test or npm run doesn't 😕

"scripts": {
  "test": "jest global"
}

@torifat
Copy link
Member Author

torifat commented Nov 24, 2016

When we run any script via npm test, it set config prefix to /usr/local (returned by config.getOption('prefix')). So, it bails out before reaching } else if (process.env.PREFIX) {.

  if (flags.prefix) {
    return flags.prefix;
  } else if (config.getOption('prefix')) {
    return String(config.getOption('prefix'));
  } else if (process.env.PREFIX) {
    return process.env.PREFIX;
  } else if (process.platform === 'win32') {

@bestander can you suggest anything here?

@onemen
Copy link

onemen commented Nov 25, 2016

@torifat I tried this and it work on my windows 10:

  • Change the test 'add with PREFIX enviroment variable' from concordant to regular.
  • Restore env before the assert.
test('add with PREFIX enviroment variable', (): Promise<void> => {
  const tmpGlobalFolder = getTempGlobalFolder();
  const envPrefix = process.env.PREFIX;
  process.env.PREFIX = tmpGlobalFolder;
  return runGlobal(['add', 'react-native-cli'], {}, 'add-with-prefix-env', async (config) => {
    // restore env
    process.env.PREFIX = envPrefix;
    assert.ok(await fs.exists(getGlobalPath(tmpGlobalFolder, 'react-native')));
  });
});

The test will still fails if user set prefix in its config file.
getGlobalPrefix function check config.getOption('prefix') before process.env.PREFIX

@torifat
Copy link
Member Author

torifat commented Nov 25, 2016

@onemen thanks. I will try it.

The test will still fails if user set prefix in its config file.

I can reset it with a .yarnrc in the fixture folder. Or, I could mock user-home module.

Restore env before the assert

I'm not sure how it helped 😕

@onemen
Copy link

onemen commented Nov 25, 2016

It work for me also on linux
Add .yarn-global/.yarnrc in __tests__/fixtures/global/add-with-prefix-env/.yarn-global/.yarnrc

with
prefix ""

@bestander bestander merged commit ac90c23 into yarnpkg:master Nov 27, 2016
@torifat torifat deleted the test/global-add-prefix branch November 27, 2016 05:09
@onemen
Copy link

onemen commented Nov 27, 2016

I'v just update to the new master with ac90c23

add with PREFIX enviroment variable test failed on Windows and Linux, when the user profile includes prefix

  • Move add-with-prefix-env/.yarnrc to add-with-prefix-env/.yarn-global/.yarnrc
  • Fix typo enviroment > environment

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 this pull request may close these issues.

None yet

3 participants