Skip to content

Commit b7a0747

Browse files
acelayaacelaya
committed
Refactor and modernize some tests
1 parent 0667e79 commit b7a0747

15 files changed

+84
-91
lines changed

‎src/annotator/config/test/config-func-settings-from-test.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { configFuncSettingsFrom } from '../config-func-settings-from';
33
describe('annotator.config.configFuncSettingsFrom', () => {
44
const sandbox = sinon.createSandbox();
55

6-
afterEach('reset the sandbox', () => {
6+
afterEach(() => {
77
sandbox.restore();
88
});
99

@@ -16,7 +16,7 @@ describe('annotator.config.configFuncSettingsFrom', () => {
1616
});
1717

1818
context("when window.hypothesisConfig() isn't a function", () => {
19-
beforeEach('stub console.warn()', () => {
19+
beforeEach(() => {
2020
sandbox.stub(console, 'warn');
2121
});
2222

‎src/annotator/config/test/index-test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ describe('annotator/config/index', () => {
5555
);
5656

5757
context("when there's no application/annotator+html <link>", () => {
58-
beforeEach('remove the application/annotator+html <link>', () => {
58+
beforeEach(() => {
5959
Object.defineProperty(fakeSettingsFrom(), 'sidebarAppUrl', {
6060
get: sinon.stub().throws(new Error("there's no link")),
6161
});

‎src/annotator/config/test/settings-test.js

+24-27
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ describe('annotator/config/settingsFrom', () => {
8585
context(
8686
'when the host page has a js-hypothesis-config with an annotations setting',
8787
() => {
88-
beforeEach('add a js-hypothesis-config annotations setting', () => {
88+
beforeEach(() => {
8989
fakeParseJsonConfig.returns({
9090
annotations: 'annotationsFromJSON',
9191
});
@@ -101,19 +101,16 @@ describe('annotator/config/settingsFrom', () => {
101101
context(
102102
"when there's also an `annotations` in the URL fragment",
103103
() => {
104-
specify(
105-
'js-hypothesis-config annotations override URL ones',
106-
() => {
107-
const window_ = fakeWindow(
108-
'http://localhost:3000#annotations:annotationsFromURL',
109-
);
110-
111-
assert.equal(
112-
settingsFrom(window_).annotations,
113-
'annotationsFromJSON',
114-
);
115-
},
116-
);
104+
it('overrides URLs with js-hypothesis-config annotations', () => {
105+
const window_ = fakeWindow(
106+
'http://localhost:3000#annotations:annotationsFromURL',
107+
);
108+
109+
assert.equal(
110+
settingsFrom(window_).annotations,
111+
'annotationsFromJSON',
112+
);
113+
});
117114
},
118115
);
119116
},
@@ -183,7 +180,7 @@ describe('annotator/config/settingsFrom', () => {
183180
context(
184181
'when the host page has a js-hypothesis-config with a query setting',
185182
() => {
186-
beforeEach('add a js-hypothesis-config query setting', () => {
183+
beforeEach(() => {
187184
fakeParseJsonConfig.returns({
188185
query: 'queryFromJSON',
189186
});
@@ -194,7 +191,7 @@ describe('annotator/config/settingsFrom', () => {
194191
});
195192

196193
context("when there's also a query in the URL fragment", () => {
197-
specify('js-hypothesis-config queries override URL ones', () => {
194+
it('overrides URL query with js-hypothesis-config ones', () => {
198195
const window_ = fakeWindow(
199196
'http://localhost:3000#annotations:query:queryFromUrl',
200197
);
@@ -338,54 +335,54 @@ describe('annotator/config/settingsFrom', () => {
338335
[
339336
{
340337
when: 'the client is embedded in a web page',
341-
specify: 'it returns setting values from window.hypothesisConfig()',
338+
title: 'returns setting values from window.hypothesisConfig()',
342339
configFuncSettings: { foo: 'configFuncValue' },
343340
jsonSettings: { foo: 'ignored' }, // hypothesisConfig() overrides js-hypothesis-config
344341
expected: 'configFuncValue',
345342
},
346343
{
347344
when: 'the client is embedded in a web page',
348-
specify:
349-
'it ignores settings from js-hypothesis-config if `ignoreOtherConfiguration` is present',
345+
title:
346+
'ignores settings from js-hypothesis-config if `ignoreOtherConfiguration` is present',
350347
isBrowserExtension: false,
351348
configFuncSettings: { ignoreOtherConfiguration: '1' },
352349
jsonSettings: { foo: 'ignored' },
353350
expected: undefined,
354351
},
355352
{
356353
when: 'the client is embedded in a web page',
357-
specify: 'it returns setting values from js-hypothesis-config objects',
354+
title: 'returns setting values from js-hypothesis-config objects',
358355
configFuncSettings: {},
359356
jsonSettings: { foo: 'jsonValue' },
360357
expected: 'jsonValue',
361358
},
362359
{
363360
when: 'the client is embedded in a web page',
364-
specify:
365-
'hypothesisConfig() settings override js-hypothesis-config ones',
361+
title:
362+
'overrides js-hypothesis-config with hypothesisConfig() settings',
366363
configFuncSettings: { foo: 'configFuncValue' },
367364
jsonSettings: { foo: 'jsonValue' },
368365
expected: 'configFuncValue',
369366
},
370367
{
371368
when: 'the client is embedded in a web page',
372-
specify:
373-
'even a null from hypothesisConfig() overrides js-hypothesis-config',
369+
title:
370+
'overrides js-hypothesis-config even with null from hypothesisConfig()',
374371
configFuncSettings: { foo: null },
375372
jsonSettings: { foo: 'jsonValue' },
376373
expected: null,
377374
},
378375
{
379376
when: 'the client is embedded in a web page',
380-
specify:
381-
'even an undefined from hypothesisConfig() overrides js-hypothesis-config',
377+
title:
378+
'overrides js-hypothesis-config even with undefined from hypothesisConfig()',
382379
configFuncSettings: { foo: undefined },
383380
jsonSettings: { foo: 'jsonValue' },
384381
expected: undefined,
385382
},
386383
].forEach(test => {
387384
context(test.when, () => {
388-
specify(test.specify, () => {
385+
it(test.title, () => {
389386
fakeConfigFuncSettingsFrom.returns(test.configFuncSettings);
390387
fakeParseJsonConfig.returns(test.jsonSettings);
391388
const settings = settingsFrom(fakeWindow());

‎src/annotator/integrations/test/pdf-metadata-test.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { delay } from '@hypothesis/frontend-testing';
22

33
import { EventEmitter } from '../../../shared/event-emitter';
4-
import { promiseWithResolvers } from '../../../shared/promise-with-resolvers';
54
import { PDFMetadata } from '../pdf-metadata';
65

76
/**
@@ -36,7 +35,7 @@ class FakePDFDocumentProxy {
3635
this._metadata = null;
3736
this.fingerprint = null;
3837

39-
const { resolve, promise } = promiseWithResolvers();
38+
const { resolve, promise } = Promise.withResolvers();
4039
this._downloadInfoResolver = resolve;
4140
this._downloadInfoPromise = promise;
4241
}

‎src/boot/test/parse-json-config-test.js

+12-15
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ describe('#parseJsonConfig', () => {
2929
});
3030

3131
context("when there's JSON scripts with no top-level objects", () => {
32-
beforeEach('add JSON scripts with no top-level objects', () => {
32+
beforeEach(() => {
3333
appendJSHypothesisConfig(document, 'null');
3434
appendJSHypothesisConfig(document, '23');
3535
appendJSHypothesisConfig(document, 'true');
@@ -41,7 +41,7 @@ describe('#parseJsonConfig', () => {
4141
});
4242

4343
context("when there's a JSON script with a top-level array", () => {
44-
beforeEach('add a JSON script containing a top-level array', () => {
44+
beforeEach(() => {
4545
appendJSHypothesisConfig(document, '["a", "b", "c"]');
4646
});
4747

@@ -51,7 +51,7 @@ describe('#parseJsonConfig', () => {
5151
});
5252

5353
context("when there's a JSON script with a top-level string", () => {
54-
beforeEach('add a JSON script with a top-level string', () => {
54+
beforeEach(() => {
5555
appendJSHypothesisConfig(document, '"hi"');
5656
});
5757

@@ -61,11 +61,11 @@ describe('#parseJsonConfig', () => {
6161
});
6262

6363
context("when there's a JSON script containing invalid JSON", () => {
64-
beforeEach('stub console.warn()', () => {
64+
beforeEach(() => {
6565
sandbox.stub(console, 'warn');
6666
});
6767

68-
beforeEach('add a JSON script containing invalid JSON', () => {
68+
beforeEach(() => {
6969
appendJSHypothesisConfig(document, 'this is not valid json');
7070
});
7171

@@ -87,7 +87,7 @@ describe('#parseJsonConfig', () => {
8787
});
8888

8989
context("when there's a JSON script with an empty object", () => {
90-
beforeEach('add a JSON script containing an empty object', () => {
90+
beforeEach(() => {
9191
appendJSHypothesisConfig(document, '{}');
9292
});
9393

@@ -97,7 +97,7 @@ describe('#parseJsonConfig', () => {
9797
});
9898

9999
context("when there's a JSON script containing some settings", () => {
100-
beforeEach('add a JSON script containing some settings', () => {
100+
beforeEach(() => {
101101
appendJSHypothesisConfig(document, '{"foo": "FOO", "bar": "BAR"}');
102102
});
103103

@@ -107,7 +107,7 @@ describe('#parseJsonConfig', () => {
107107
});
108108

109109
context('when there are JSON scripts with different settings', () => {
110-
beforeEach('add some JSON scripts with different settings', () => {
110+
beforeEach(() => {
111111
appendJSHypothesisConfig(document, '{"foo": "FOO"}');
112112
appendJSHypothesisConfig(document, '{"bar": "BAR"}');
113113
appendJSHypothesisConfig(document, '{"gar": "GAR"}');
@@ -123,17 +123,14 @@ describe('#parseJsonConfig', () => {
123123
});
124124

125125
context('when multiple JSON scripts contain the same setting', () => {
126-
beforeEach('add some JSON scripts with different settings', () => {
126+
beforeEach(() => {
127127
appendJSHypothesisConfig(document, '{"foo": "first"}');
128128
appendJSHypothesisConfig(document, '{"foo": "second"}');
129129
appendJSHypothesisConfig(document, '{"foo": "third"}');
130130
});
131131

132-
specify(
133-
'settings from later in the page override ones from earlier',
134-
() => {
135-
assert.equal(parseJsonConfig(document).foo, 'third');
136-
},
137-
);
132+
it('overrides settings from earlier in the page with later ones', () => {
133+
assert.equal(parseJsonConfig(document).foo, 'third');
134+
});
138135
});
139136
});

‎src/shared/messaging/test/port-rpc-test.js

+18-14
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,12 @@ describe('PortRPC', () => {
7979
sinon.restore();
8080
});
8181

82-
it('should call the method `plusOne` on rpc2', done => {
83-
rpc1.call('plusOne', 1, 2, 3, value => {
84-
assert.deepEqual(value, [2, 3, 4]);
85-
done();
86-
});
82+
it('should call the method `plusOne` on rpc2', async () => {
83+
const { promise, resolve } = Promise.withResolvers();
84+
rpc1.call('plusOne', 1, 2, 3, resolve);
85+
86+
const value = await promise;
87+
assert.deepEqual(value, [2, 3, 4]);
8788
});
8889

8990
it('should not call the method `plusOne` if rpc1 is destroyed', () => {
@@ -102,15 +103,18 @@ describe('PortRPC', () => {
102103
assert.notCalled(plusOne);
103104
});
104105

105-
it('should call the method `concat` on rpc1', done => {
106-
rpc2.call('concat', 'hello', ' ', 'world', value => {
107-
assert.equal(value, 'hello world');
108-
});
109-
110-
rpc2.call('concat', [1], [2], [3], value => {
111-
assert.deepEqual(value, [1, 2, 3]);
112-
done();
113-
});
106+
it('should call the method `concat` on rpc1', async () => {
107+
const { promise: promiseOne, resolve: resolvePromiseOne } =
108+
Promise.withResolvers();
109+
rpc2.call('concat', 'hello', ' ', 'world', resolvePromiseOne);
110+
const valueOne = await promiseOne;
111+
assert.equal(valueOne, 'hello world');
112+
113+
const { promise: promiseTwo, resolve: resolvePromiseTwo } =
114+
Promise.withResolvers();
115+
rpc2.call('concat', [1], [2], [3], resolvePromiseTwo);
116+
const valueTwo = await promiseTwo;
117+
assert.deepEqual(valueTwo, [1, 2, 3]);
114118
});
115119

116120
it('should call method on valid message', async () => {

‎src/shared/test/cfi-test.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ describe('sidebar/util/cfi', () => {
4141
expected: ['/2/4', '/2/4'],
4242
},
4343
].forEach(({ range, expected }) => {
44-
assert.deepEqual(splitCFIRange(range), expected);
44+
it('splits range into expected pieces', () => {
45+
assert.deepEqual(splitCFIRange(range), expected);
46+
});
4547
});
4648
});
4749

‎src/shared/test/promise-with-resolvers-test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { promiseWithResolvers } from '../promise-with-resolvers';
22

33
describe('promiseWithResolvers', () => {
44
it('resolves returned promise with `resolve` callback', async () => {
5-
const { promise, resolve } = promiseWithResolvers();
5+
const { promise, resolve } = Promise.withResolvers();
66
const expected = 'some value';
77

88
resolve(expected);

‎src/sidebar/components/test/HypothesisApp-test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ describe('HypothesisApp', () => {
343343
});
344344

345345
context('when a third-party service is in use', () => {
346-
beforeEach('configure a third-party service to be in use', () => {
346+
beforeEach(() => {
347347
fakeServiceConfig.returns({});
348348
});
349349

‎src/sidebar/components/test/ModerationBanner-test.js

+10-13
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
checkAccessibility,
33
mockImportedComponents,
4+
waitFor,
45
} from '@hypothesis/frontend-testing';
56
import { mount } from '@hypothesis/frontend-testing';
67

@@ -121,7 +122,7 @@ describe('ModerationBanner', () => {
121122
assert.calledWith(fakeApi.annotation.hide, { id: 'ann-id' });
122123
});
123124

124-
it('reports an error if hiding the annotation fails', done => {
125+
it('reports an error if hiding the annotation fails', async () => {
125126
const wrapper = createComponent({
126127
annotation: moderatedAnnotation({
127128
flagCount: 10,
@@ -130,10 +131,9 @@ describe('ModerationBanner', () => {
130131
fakeApi.annotation.hide.returns(Promise.reject(new Error('Network Error')));
131132
wrapper.find('button').simulate('click');
132133

133-
setTimeout(() => {
134-
assert.calledWith(fakeToastMessenger.error, 'Failed to hide annotation');
135-
done();
136-
}, 0);
134+
await waitFor(() =>
135+
fakeToastMessenger.error.calledWith('Failed to hide annotation'),
136+
);
137137
});
138138

139139
it('unhides the annotation if "Unhide" is clicked', () => {
@@ -147,7 +147,7 @@ describe('ModerationBanner', () => {
147147
assert.calledWith(fakeApi.annotation.unhide, { id: 'ann-id' });
148148
});
149149

150-
it('reports an error if unhiding the annotation fails', done => {
150+
it('reports an error if unhiding the annotation fails', async () => {
151151
const wrapper = createComponent({
152152
annotation: moderatedAnnotation({
153153
flagCount: 1,
@@ -158,13 +158,10 @@ describe('ModerationBanner', () => {
158158
Promise.reject(new Error('Network Error')),
159159
);
160160
wrapper.find('button').simulate('click');
161-
setTimeout(() => {
162-
assert.calledWith(
163-
fakeToastMessenger.error,
164-
'Failed to unhide annotation',
165-
);
166-
done();
167-
}, 0);
161+
162+
await waitFor(() =>
163+
fakeToastMessenger.error.calledWith('Failed to unhide annotation'),
164+
);
168165
});
169166

170167
it(

0 commit comments

Comments
 (0)