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

Show warning in cli in case of multiple configs #10213

Closed
wants to merge 4 commits into from

Conversation

Mansehej
Copy link

Summary

Helps the user have correct and consistent configuration and helps when trying to debug why config is not applied.

As mentioned in #10124 and #10123.

Warns in case of multiple configs, however the warning is overriden in case of the usage of --config.

Test plan

Tested it locally by trying a development build of jest in another package.
However, I'm unsure of writing the tests with respect to this functionality.
If someone could point me in the right direction (in case tests are necessary for this use case), it would be really helpful.

@Mansehej Mansehej changed the title Show error in cli in case of multiple configs Show warning in cli in case of multiple configs Jun 27, 2020
Mansehej added a commit to Mansehej/jest that referenced this pull request Jun 27, 2020
Show warning in cli in case of multiple configs
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great, thanks! Could you add an e2e test that verifies the error triggers?

CHANGELOG.md Outdated
@@ -8,6 +8,12 @@

### Performance

## 26.1.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just keep it under master - I'll add a version here when releasing 👍

@Mansehej
Copy link
Author

Mansehej commented Jul 6, 2020

Hey @SimenB,

I made an empty jest-config.js in the tests directory and then added snapshot tests.
I am not quite sure if this is the correct way to do it.

Could you please let me know if this is fine, so that it can be either merged/ I can edit it accordingly.

Thanks,
Mansehej.

@Mansehej Mansehej requested a review from SimenB July 6, 2020 17:57
@Mansehej
Copy link
Author

Hey @SimenB
Did you get a chance to take a look at this?

Thanks,
Mansehej.

@SimenB
Copy link
Member

SimenB commented Jul 24, 2020

@Mansehej sorry about the slow response! I've been away on holiday. I'll take a look at this next week when I'm back at work 🙂

@Mansehej
Copy link
Author

@SimenB Oh no issues, have fun!

@SimenB
Copy link
Member

SimenB commented Oct 31, 2020

I completely forgot about this, sorry about that @Mansehej! Should have created a reminder for myself 😬

What I would do is create some temporary directories and write config into them, sorta like https://github.com/facebook/jest/blob/a94730c685a40055fe1694d7e4833061d0055f79/e2e/__tests__/dependencyClash.test.ts#L40-L45

So something like

/**
 * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

import * as path from 'path';
import {tmpdir} from 'os';
import {cleanup, createEmptyPackage, writeFiles} from '../Utils';
import runJest from '../runJest';

const tempDir = path.resolve(tmpdir(), 'multiple-jest-configs');

beforeEach(() => {
  cleanup(tempDir);
});

test('throws when jest.config.js and package.json config exist', () => {
  createEmptyPackage(tempDir);
  writeFiles(tempDir, {
    '__tests__/test.js': `
      test('dummy test', () => expect(1).toBe(1));
    `,
    'jest.config.js': `module.exports = {
    };`,
  });
  const {stderr, exitCode} = runJest(tempDir);

  expect(exitCode).toBe(1);
  expect(stderr); // something - probably a `toContain` on the error text
});

test('does not throw when jest.config.js exist, but no config in package.json', () => {
  createEmptyPackage(tempDir, {
    description: 'THIS IS AN AUTOGENERATED FILE AND SHOULD NOT BE ADDED TO GIT',
  });
  writeFiles(tempDir, {
    '__tests__/test.js': `
      test('dummy test', () => expect(1).toBe(1));
    `,
    'jest.config.js': `module.exports = {
    };`,
  });
  const {stderr, exitCode} = runJest(tempDir);

  expect(exitCode).toBe(0);
});

Then I would write tests for all the different config files we have.

@github-actions
Copy link

github-actions bot commented Nov 6, 2021

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants