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 color to displayName in project configuration. #8025

Merged
merged 32 commits into from Mar 26, 2019
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
e3bf4a3
WIP
natealcedo Feb 24, 2019
eefc4ff
Update jest-types to have displayNameColor
natealcedo Feb 28, 2019
e0f09e2
Update default value to white
natealcedo Feb 28, 2019
ec8b7c9
Add displayNameColor to Initial Options
natealcedo Feb 28, 2019
7bb02fc
Merge remote-tracking branch 'origin/master' into feature/support-dis…
natealcedo Mar 23, 2019
a810c31
Delete Config.js
natealcedo Mar 23, 2019
789cb8b
Remove displayNameColor
natealcedo Mar 23, 2019
0903cd5
Eslint fix
natealcedo Mar 23, 2019
e369ca5
Update implementation of displayName
natealcedo Mar 23, 2019
01d8d97
Update DisplayName type
natealcedo Mar 23, 2019
94caad6
NormalizedisplayName
natealcedo Mar 23, 2019
dc7b046
Add validation for when displayName is an object
natealcedo Mar 23, 2019
f75bb90
Add test cases for normalizing of displayName when the value is an ob…
natealcedo Mar 23, 2019
2a56599
Remove validation logic from utils
natealcedo Mar 23, 2019
d42b2a3
Add all colors
natealcedo Mar 24, 2019
81fc522
Use jest-get-type
natealcedo Mar 24, 2019
65aa738
Add tests to displayName
natealcedo Mar 25, 2019
041a81d
Update ProjectConfig
natealcedo Mar 25, 2019
01fd4aa
Run lint fix
natealcedo Mar 25, 2019
44429c8
Revert typing error
natealcedo Mar 25, 2019
ec0d0d0
Merge branch 'master' into feature/support-displayNameColor
natealcedo Mar 26, 2019
68a6ae4
Remove test scenario which will never happen
natealcedo Mar 26, 2019
d1fe278
Merge remote-tracking branch 'origin/master' into feature/support-dis…
natealcedo Mar 26, 2019
87dc8cf
Update docs with new displayName behavior
natealcedo Mar 26, 2019
f1dbd72
Update versioned docs
natealcedo Mar 26, 2019
aada1fb
Update changelog
natealcedo Mar 26, 2019
bea007a
Update snapshot
natealcedo Mar 26, 2019
8928a2e
Lint fix
natealcedo Mar 26, 2019
c1e5a38
Lint md fix
natealcedo Mar 26, 2019
4462019
Respond to feedback
natealcedo Mar 26, 2019
21b99bb
Add new line
natealcedo Mar 26, 2019
3665d38
Update snapshot
natealcedo Mar 26, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/jest-config/src/ValidConfig.ts
Expand Up @@ -41,7 +41,11 @@ const initialOptions: Config.InitialOptions = {
// @ts-ignore: Missing from initial options... https://github.com/facebook/jest/pull/7923
cwd: '/root',
dependencyExtractor: '<rootDir>/dependencyExtractor.js',
displayName: 'project-name',
// @ts-ignore TODO: type this properly
displayName: multipleValidOptions('test-config', {
color: 'blue',
name: 'test-config',
}),
errorOnDeprecated: false,
expand: false,
extraGlobals: [],
Expand Down
Expand Up @@ -17,6 +17,66 @@ exports[`Upgrade help logs a warning when \`scriptPreprocessor\` and/or \`prepro
<yellow></>"
`;

exports[`displayName should throw an error when displayName is is an empty object 1`] = `
"<red><bold><bold>● <bold>Validation Error</>:</>
<red></>
<red> Option \\"<bold>displayName</>\\" must be of type</>
<red> {</>
<red> name: string;</>
<red> color: string;</>
<red> }</>
<red></>
<red></>
<red> <bold>Configuration Documentation:</></>
<red> https://jestjs.io/docs/configuration.html</>
<red></>"
`;

exports[`displayName should throw an error when displayName is missing color 1`] = `
"<red><bold><bold>● <bold>Validation Error</>:</>
<red></>
<red> Option \\"<bold>displayName</>\\" must be of type</>
<red> {</>
<red> name: string;</>
<red> color: string;</>
<red> }</>
<red></>
<red></>
<red> <bold>Configuration Documentation:</></>
<red> https://jestjs.io/docs/configuration.html</>
<red></>"
`;

exports[`displayName should throw an error when displayName is missing name 1`] = `
"<red><bold><bold>● <bold>Validation Error</>:</>
<red></>
<red> Option \\"<bold>displayName</>\\" must be of type</>
<red> {</>
<red> name: string;</>
<red> color: string;</>
<red> }</>
<red></>
<red></>
<red> <bold>Configuration Documentation:</></>
<red> https://jestjs.io/docs/configuration.html</>
<red></>"
`;

exports[`displayName should throw an error when displayName is using invalid values 1`] = `
"<red><bold><bold>● <bold>Validation Error</>:</>
<red></>
<red> Option \\"<bold>displayName</>\\" must be of type</>
<red> {</>
<red> name: string;</>
<red> color: string;</>
<red> }</>
<red></>
<red></>
<red> <bold>Configuration Documentation:</></>
<red> https://jestjs.io/docs/configuration.html</>
<red></>"
`;

exports[`preset throws when module was found but no "jest-preset.js" or "jest-preset.json" files 1`] = `
"<red><bold><bold>● <bold>Validation Error</>:</>
<red></>
Expand Down
23 changes: 23 additions & 0 deletions packages/jest-config/src/__tests__/normalize.test.js
Expand Up @@ -1519,3 +1519,26 @@ describe('Defaults', () => {
expect(console.warn).not.toHaveBeenCalled();
});
});

describe('displayName', () => {
test.each`
displayName | description
${{}} | ${'is an empty object'}
${{name: 'hello'}} | ${'missing color'}
${{color: 'green'}} | ${'missing name'}
${{color: 2, name: []}} | ${'using invalid values'}
`(
'should throw an error when displayName is $description',
({displayName}) => {
expect(() => {
normalize(
{
rootDir: '/root/',
displayName,
},
{},
);
}).toThrowErrorMatchingSnapshot();
},
);
});
47 changes: 46 additions & 1 deletion packages/jest-config/src/normalize.ts
Expand Up @@ -745,6 +745,52 @@ export default function normalize(
}
break;
}
case 'displayName': {
const displayName = oldOptions[key] as Config.DisplayName;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to cast this. Typescript was complaining alot

const isDisplayNameAnObject =
SimenB marked this conversation as resolved.
Show resolved Hide resolved
typeof displayName === 'object' && !Array.isArray(displayName);
if (typeof displayName === 'string') {
value = displayName;
break;
}
/**
* Ensuring that displayName shape is correct here so that the
* reporters can trust the shape of the data
* TODO: Normalize "displayName" such that given a config option
* {
* "displayName": "Test"
* }
* becomes
* {
* displayName: {
* name: "Test",
* color: "white"
* }
* }
*
* This can't be done now since this will be a breaking change
* for custom reporters
*/
if (isDisplayNameAnObject) {
const errorMessage =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opting to do a custom error here. Didn't want to overcomplicate it. Open for suggestions here

` Option "${chalk.bold('displayName')}" must be of type\n` +
' {\n' +
' name: string;\n' +
' color: string;\n' +
' }\n';
const {name, color} = displayName;
if (
!name ||
!color ||
typeof name !== 'string' ||
typeof color !== 'string'
) {
throw createConfigError(errorMessage);
}
}
value = oldOptions[key];
break;
}
case 'automock':
case 'browser':
case 'cache':
Expand All @@ -756,7 +802,6 @@ export default function normalize(
case 'coverageThreshold':
case 'detectLeaks':
case 'detectOpenHandles':
case 'displayName':
case 'errorOnDeprecated':
case 'expand':
case 'extraGlobals':
Expand Down
16 changes: 11 additions & 5 deletions packages/jest-reporters/src/utils.ts
Expand Up @@ -17,14 +17,20 @@ const PROGRESS_BAR_WIDTH = 40;

export const printDisplayName = (config: Config.ProjectConfig) => {
const {displayName} = config;
const white = chalk.reset.inverse.white;
if (!displayName) {
return '';
}

if (displayName) {
return chalk.supportsColor
? chalk.reset.inverse.white(` ${displayName} `)
: displayName;
if (typeof displayName === 'string') {
return chalk.supportsColor ? white(` ${displayName} `) : displayName;
}

return '';
const {name, color} = displayName;
const chosenColor = chalk.reset.inverse[color]
? chalk.reset.inverse[color]
: white;
return chalk.supportsColor ? chosenColor(` ${name} `) : name;
};

export const trimAndFormatPath = (
Expand Down
3 changes: 2 additions & 1 deletion packages/jest-test-result/src/types.ts
Expand Up @@ -8,6 +8,7 @@
// eslint-disable-next-line import/no-extraneous-dependencies
import {CoverageMap, CoverageMapData} from 'istanbul-lib-coverage';
import {ConsoleBuffer} from '@jest/console';
import {Config} from '@jest/types';

export type SerializableError = {
code?: unknown;
Expand Down Expand Up @@ -102,7 +103,7 @@ export type Suite = {
export type TestResult = {
console?: ConsoleBuffer | null;
coverage?: CoverageMapData;
displayName?: string | null;
displayName?: Config.DisplayName;
failureMessage?: string | null;
leaks: boolean;
memoryUsage?: Bytes;
Expand Down
52 changes: 50 additions & 2 deletions packages/jest-types/src/Config.ts
Expand Up @@ -105,6 +105,13 @@ export type DefaultOptions = {
watchman: boolean;
};

export type DisplayName =
| string
| {
name: string;
color: DisplayNameColor;
};

export type InitialOptions = {
automock?: boolean;
bail?: boolean | number;
Expand All @@ -130,7 +137,7 @@ export type InitialOptions = {
dependencyExtractor?: string;
detectLeaks?: boolean;
detectOpenHandles?: boolean;
displayName?: string;
displayName?: DisplayName;
expand?: boolean;
extraGlobals?: Array<string>;
filter?: Path;
Expand Down Expand Up @@ -224,6 +231,47 @@ type NotifyMode =
| 'success-change'
| 'failure-change';

/**
* Hard coding this until
* https://github.com/chalk/chalk/pull/336
* gets merged
*/
type DisplayNameColor =
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to extract these from chalk instead of hard coding them

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I was hard coding this because of what I mentioned about not being sure if we should curate the colors that we should allow or if we should allow everything. Hence the early PR 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've taken a look at the types provided by chalk and they don't expose the colors. https://github.com/chalk/chalk/blob/master/index.d.ts#L231

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just as an FYI, opened up this PR. Hopefully this gets merged and we won't have to hardcode colors here. chalk/chalk#336

| 'black'
| 'red'
| 'green'
| 'yellow'
| 'blue'
| 'magenta'
| 'cyan'
| 'white'
| 'gray'
| 'grey'
| 'blackBright'
| 'redBright'
| 'greenBright'
| 'yellowBright'
| 'blueBright'
| 'magentaBright'
| 'cyanBright'
| 'whiteBright'
| 'bgBlack'
| 'bgRed'
| 'bgGreen'
| 'bgYellow'
| 'bgBlue'
| 'bgMagenta'
| 'bgCyan'
| 'bgWhite'
| 'bgBlackBright'
| 'bgRedBright'
| 'bgGreenBright'
| 'bgYellowBright'
| 'bgBlueBright'
| 'bgMagentaBright'
| 'bgCyanBright'
| 'bgWhiteBright';

type CoverageThreshold = {
[path: string]: {
[key: string]: number;
Expand Down Expand Up @@ -319,7 +367,7 @@ export type ProjectConfig = {
dependencyExtractor?: string;
detectLeaks: boolean;
detectOpenHandles: boolean;
displayName: string | null | undefined;
displayName?: DisplayName;
errorOnDeprecated: boolean;
extraGlobals: Array<keyof NodeJS.Global>;
filter: Path | null | undefined;
Expand Down