Skip to content

Commit 186c05a

Browse files
fix: correct tests for createAppointment
Corrects the test so that they respect locales with Daylight Savings Time. Previously, tests and suggested implementation simply did simple time arithmetic based on the offset provided to `createAppointment`. This is wrong, as it will shift appointment time-of-day when moving accross DST boundaries. This changes test implementation so that: 1. correct usage of input times is checked, by passing a 0 offset 2. correct offsetting of appointment time is checked, by passing in a known start date and then creating one appointment that is within the same DST state and one that is not. re: https://forum.exercism.org/t/tests-and-suggested-implementation-for-appointment-time-are-wrong/
1 parent 709a441 commit 186c05a

File tree

1 file changed

+21
-17
lines changed

1 file changed

+21
-17
lines changed

exercises/concept/appointment-time/appointment-time.spec.js

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,34 +10,38 @@ import {
1010
} from './appointment-time';
1111

1212
describe('createAppointment', () => {
13-
test('creates appointment 4 days in the future', () => {
14-
const currentTime = Date.now();
15-
const expectedTime = currentTime + 345600 * 1000;
13+
test('uses the passed in current time', () => {
14+
const currentTime = Date.UTC(2000, 6, 16, 12, 0, 0, 0);
15+
const result = createAppointment(0, currentTime);
1616

17-
expect(createAppointment(4, currentTime)).toEqual(new Date(expectedTime));
17+
expect(result).toEqual(new Date(currentTime));
1818
});
1919

20-
test('creates appointment 124 in the future', () => {
20+
test('uses the actual current time when it is not passed in', () => {
2121
const currentTime = Date.now();
22-
const expectedTime = currentTime + 10713600 * 1000;
22+
const result = createAppointment(0);
2323

24-
expect(createAppointment(124, currentTime)).toEqual(new Date(expectedTime));
24+
expect(result).toEqual(new Date(currentTime));
2525
});
2626

27-
test('uses the passed in current time', () => {
28-
const currentTime = Date.UTC(2000, 6, 16, 12, 0, 0, 0);
29-
const result = createAppointment(0, currentTime);
27+
test('creates appointment without DST change', () => {
28+
const offset = 4; // days
3029

31-
expect(result.getFullYear()).toEqual(2000);
30+
const currentTime = Date.UTC(2000, 6, 1, 12, 0, 0, 0);
31+
const expectedTime = currentTime + offset * 24 * 60 * 60 * 1000;
32+
33+
expect(createAppointment(offset, currentTime)).toEqual(new Date(expectedTime));
3234
});
3335

34-
test('uses the actual current time when it is not passed in', () => {
35-
const result = createAppointment(0);
36+
test('creates appointment with potential DST change', () => {
37+
const offset = 180; // days
3638

37-
expect(Math.abs(Date.now() - result.getTime())).toBeLessThanOrEqual(
38-
// Maximum number of time zones difference
39-
27 * 60 * 60 * 1000,
40-
);
39+
const currentTime = Date.UTC(2000, 6, 1, 12, 0, 0, 0);
40+
let expectedTime = currentTime + offset * 24 * 60 * 60 * 1000;
41+
// Manually adjust for DST timezone offset:
42+
expectedTime += (new Date(expectedTime).getTimezoneOffset() - new Date(currentTime).getTimezoneOffset()) * 60 * 1000;
43+
44+
expect(createAppointment(offset, currentTime)).toEqual(new Date(expectedTime));
4145
});
4246

4347
test('rolls over days, months, and years', () => {

0 commit comments

Comments
 (0)