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

Replace hex with utf8 #6253

Merged
merged 5 commits into from
May 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@

### Fixes

* `[jest-cli]` Fix stdin encoding to utf8 for watch plugins.
([#6253](https://github.com/facebook/jest/issues/6253))
* `[expect]` Better detection of DOM Nodes for equality
([#6246](https://github.com/facebook/jest/pull/6246))
* `[jest-cli]` Fix misleading action description for F key when in "only failed
Expand Down
31 changes: 15 additions & 16 deletions packages/jest-cli/src/__tests__/watch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ jest.doMock(
class WatchPlugin1 {
getUsageInfo() {
return {
key: 's'.codePointAt(0),
key: 's',
prompt: 'do nothing',
};
}
Expand All @@ -79,7 +79,7 @@ jest.doMock(
class WatchPlugin2 {
getUsageInfo() {
return {
key: 'u'.codePointAt(0),
key: 'u',
prompt: 'do something else',
};
}
Expand All @@ -90,7 +90,6 @@ jest.doMock(
const watch = require('../watch').default;

const nextTick = () => new Promise(res => process.nextTick(res));
const toHex = char => Number(char.charCodeAt(0)).toString(16);

afterEach(runJestMock.mockReset);

Expand Down Expand Up @@ -329,7 +328,7 @@ describe('Watch mode flows', () => {
}
getUsageInfo() {
return {
key: 'p'.codePointAt(0),
key: 'p',
prompt: 'custom "P" plugin',
};
}
Expand All @@ -352,7 +351,7 @@ describe('Watch mode flows', () => {

expect(pipe.write.mock.calls.reverse()[0]).toMatchSnapshot();

stdin.emit(toHex('p'));
stdin.emit('p');
await nextTick();

expect(run).toHaveBeenCalled();
Expand Down Expand Up @@ -405,7 +404,7 @@ describe('Watch mode flows', () => {
}
getUsageInfo() {
return {
key: 's'.codePointAt(0),
key: 's',
prompt: 'do nothing',
};
}
Expand All @@ -424,7 +423,7 @@ describe('Watch mode flows', () => {
stdin,
);

stdin.emit(Number('s'.charCodeAt(0)).toString(16));
stdin.emit('s');

await nextTick();

Expand All @@ -445,7 +444,7 @@ describe('Watch mode flows', () => {
onKey() {}
getUsageInfo() {
return {
key: 's'.codePointAt(0),
key: 's',
prompt: 'do nothing',
};
}
Expand All @@ -465,7 +464,7 @@ describe('Watch mode flows', () => {
onKey() {}
getUsageInfo() {
return {
key: 'z'.codePointAt(0),
key: 'z',
prompt: 'also do nothing',
};
}
Expand All @@ -484,14 +483,14 @@ describe('Watch mode flows', () => {
stdin,
);

stdin.emit(Number('s'.charCodeAt(0)).toString(16));
stdin.emit('s');
await nextTick();
expect(run).toHaveBeenCalled();
stdin.emit(Number('z'.charCodeAt(0)).toString(16));
stdin.emit('z');
await nextTick();
expect(showPrompt2).not.toHaveBeenCalled();
await resolveShowPrompt();
stdin.emit(Number('z'.charCodeAt(0)).toString(16));
stdin.emit('z');
expect(showPrompt2).toHaveBeenCalled();
});

Expand Down Expand Up @@ -537,7 +536,7 @@ describe('Watch mode flows', () => {
runJestMock.mockReset();

stdin.emit(KEYS.T);
['t', 'e', 's', 't'].map(toHex).forEach(key => stdin.emit(key));
['t', 'e', 's', 't'].forEach(key => stdin.emit(key));
stdin.emit(KEYS.ENTER);
await nextTick();

Expand All @@ -556,7 +555,7 @@ describe('Watch mode flows', () => {
runJestMock.mockReset();

stdin.emit(KEYS.P);
['f', 'i', 'l', 'e'].map(toHex).forEach(key => stdin.emit(key));
['f', 'i', 'l', 'e'].forEach(key => stdin.emit(key));
stdin.emit(KEYS.ENTER);
await nextTick();

Expand All @@ -575,12 +574,12 @@ describe('Watch mode flows', () => {
runJestMock.mockReset();

stdin.emit(KEYS.P);
['f', 'i', 'l', 'e'].map(toHex).forEach(key => stdin.emit(key));
['f', 'i', 'l', 'e'].forEach(key => stdin.emit(key));
stdin.emit(KEYS.ENTER);
await nextTick();

stdin.emit(KEYS.T);
['t', 'e', 's', 't'].map(toHex).forEach(key => stdin.emit(key));
['t', 'e', 's', 't'].forEach(key => stdin.emit(key));
stdin.emit(KEYS.ENTER);
await nextTick();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ const watch = require('../watch').default;

const nextTick = () => new Promise(res => process.nextTick(res));

const toHex = char => Number(char.charCodeAt(0)).toString(16);

const globalConfig = {watch: true};

afterEach(runJestMock.mockReset);
Expand Down Expand Up @@ -122,11 +120,11 @@ describe('Watch mode flows', () => {
};

// Write a pattern
['p', '.', '*', '1', '0'].map(toHex).forEach(assertPattern);
['p', '.', '*', '1', '0'].forEach(assertPattern);

[KEYS.BACKSPACE, KEYS.BACKSPACE].forEach(assertPattern);

['3'].map(toHex).forEach(assertPattern);
['3'].forEach(assertPattern);

// Runs Jest again
runJestMock.mockReset();
Expand All @@ -152,17 +150,14 @@ describe('Watch mode flows', () => {
await nextTick();

['p', '.', '*', '1', '0']
.map(toHex)

Copy link
Member

Choose a reason for hiding this comment

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

blank line? Seems odd prettier doesn't kill it

.concat(KEYS.ENTER)
.forEach(key => stdin.emit(key));

stdin.emit(KEYS.T);
await nextTick();

['t', 'e', 's', 't']
.map(toHex)
.concat(KEYS.ENTER)
.forEach(key => stdin.emit(key));
['t', 'e', 's', 't'].concat(KEYS.ENTER).forEach(key => stdin.emit(key));

await nextTick();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ jest.doMock('../lib/terminal_utils', () => ({

const watch = require('../watch').default;

const toHex = char => Number(char.charCodeAt(0)).toString(16);

const globalConfig = {
watch: true,
};
Expand Down Expand Up @@ -136,11 +134,11 @@ describe('Watch mode flows', () => {
};

// Write a pattern
['c', 'o', 'n', ' ', '1', '2'].map(toHex).forEach(assertPattern);
['c', 'o', 'n', ' ', '1', '2'].forEach(assertPattern);

[KEYS.BACKSPACE, KEYS.BACKSPACE].forEach(assertPattern);

['*'].map(toHex).forEach(assertPattern);
['*'].forEach(assertPattern);

// Runs Jest again
runJestMock.mockReset();
Expand Down
46 changes: 24 additions & 22 deletions packages/jest-cli/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,30 @@ const isWindows = process.platform === 'win32';
export const CLEAR = isWindows ? '\x1B[2J\x1B[0f' : '\x1B[2J\x1B[3J\x1B[H';
export const ARROW = ' \u203A ';
export const KEYS = {
A: '61',
ARROW_DOWN: '1b5b42',
ARROW_LEFT: '1b5b44',
ARROW_RIGHT: '1b5b43',
ARROW_UP: '1b5b41',
BACKSPACE: isWindows ? '08' : '7f',
C: '63',
CONTROL_C: '03',
CONTROL_D: '04',
ENTER: '0d',
ESCAPE: '1b',
F: '66',
I: '69',
O: '6f',
P: '70',
Q: '71',
QUESTION_MARK: '3f',
R: '72',
S: '73',
T: '74',
U: '75',
W: '77',
A: 'a',
Copy link
Member

Choose a reason for hiding this comment

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

I think we should remove all these, but no biggie. Can clean up later

ARROW_DOWN: '\u001b[B',
ARROW_LEFT: '\u001b[D',
ARROW_RIGHT: '\u001b[C',
ARROW_UP: '\u001b[A',
BACKSPACE: isWindows
? Buffer.from('08', 'hex').toString()
: Buffer.from('7f', 'hex').toString(),
C: 'c',
CONTROL_C: '\u0003',
CONTROL_D: '\u0004',
ENTER: '\r',
ESCAPE: '\u001b',
F: 'f',
I: 'i',
O: 'o',
P: 'p',
Q: 'q',
QUESTION_MARK: '?',
R: 'r',
S: 's',
T: 't',
U: 'u',
W: 'w',
};

export const ICONS = {
Expand Down
6 changes: 1 addition & 5 deletions packages/jest-cli/src/lib/Prompt.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,8 @@ export default class Prompt {
case KEYS.ARROW_RIGHT:
break;
default:
const char = new Buffer(key, 'hex').toString();

this._value =
key === KEYS.BACKSPACE
? this._value.slice(0, -1)
: this._value + char;
key === KEYS.BACKSPACE ? this._value.slice(0, -1) : this._value + key;
this._offset = -1;
this._selection = null;
this._onChange();
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-cli/src/lib/__tests__/prompt.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import Prompt from '../Prompt';
import {KEYS} from '../../constants';

const EXTRA_KEYS = Object.assign({}, KEYS, {
E: '65',
S: '73',
E: 'e',
S: 's',
});

it('calls handler on change value', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-cli/src/lib/handle_deprecation_warnings.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default (
// $FlowFixMe
stdin.setRawMode(true);
stdin.resume();
stdin.setEncoding('hex');
stdin.setEncoding('utf8');
stdin.on('data', (key: string) => {
if (key === KEYS.ENTER) {
resolve();
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-cli/src/lib/watch_plugins_helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const getSortedUsageRows = (
const usageInfoB = b.getUsageInfo && b.getUsageInfo(globalConfig);

if (usageInfoA && usageInfoB) {
return usageInfoA.key - usageInfoB.key;
return usageInfoA.key.localeCompare(usageInfoB.key);
}

return 0;
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-cli/src/plugins/quit.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class QuitPlugin extends BaseWatchPlugin {

getUsageInfo() {
return {
key: 'q'.codePointAt(0),
key: 'q',
prompt: 'quit watch mode',
};
}
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-cli/src/plugins/test_name_pattern.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class TestNamePatternPlugin extends BaseWatchPlugin {

getUsageInfo() {
return {
key: 't'.codePointAt(0),
key: 't',
prompt: 'filter by a test name regex pattern',
};
}
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-cli/src/plugins/test_path_pattern.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class TestPathPatternPlugin extends BaseWatchPlugin {

getUsageInfo() {
return {
key: 'p'.codePointAt(0),
key: 'p',
prompt: 'filter by a filename regex pattern',
};
}
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-cli/src/plugins/update_snapshots.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class UpdateSnapshotsPlugin extends BaseWatchPlugin {
getUsageInfo(globalConfig: GlobalConfig) {
if (this._hasSnapshotFailure) {
return {
key: 'u'.codePointAt(0),
key: 'u',
prompt: 'update failing snapshots',
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class UpdateSnapshotInteractivePlugin extends BaseWatchPlugin {
this._failedSnapshotTestAssertions.length > 0
) {
return {
key: 'i'.codePointAt(0),
key: 'i',
prompt: 'update failing snapshots interactively',
};
}
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-cli/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type {GlobalConfig} from 'types/Config';
import type {JestHookSubscriber} from './jest_hooks';

export type UsageData = {
key: number,
key: string,
prompt: string,
};

Expand Down
6 changes: 3 additions & 3 deletions packages/jest-cli/src/watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ export default function watch(
).find(plugin => {
const usageData =
(plugin.getUsageInfo && plugin.getUsageInfo(globalConfig)) || {};
return usageData.key === parseInt(key, 16);
return usageData.key === key;
});

if (matchingWatchPlugin != null) {
Expand Down Expand Up @@ -359,7 +359,7 @@ export default function watch(
if (typeof stdin.setRawMode === 'function') {
stdin.setRawMode(true);
stdin.resume();
stdin.setEncoding('hex');
stdin.setEncoding('utf8');
stdin.on('data', onKeypress);
}

Expand Down Expand Up @@ -405,7 +405,7 @@ const usage = (
plugin =>
chalk.dim(' \u203A Press') +
' ' +
String.fromCodePoint(plugin.key) +
plugin.key +
' ' +
chalk.dim(`to ${plugin.prompt}.`),
),
Expand Down