Skip to content

Commit a5bd153

Browse files
BadIdeaExceptionCool-KattSleeplessByte
authored
Fix faulty tests for createAppointment (#2767)
* fix: correct implementation of `createAppointment` Makes the suggested implementation of `createAppointment` correctly handle appointment creations that cross timezones - e.g. due to Daylight Savings Time. re: https://forum.exercism.org/t/tests-and-suggested-implementation-for-appointment-time-are-wrong/ * 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/ * docs: update hints * chore: update contributors * style: prettier * docs: refer to instructions for going about getter method Co-authored-by: Cool-Katt <[email protected]> * docs: more concise hint about setters Co-authored-by: Cool-Katt <[email protected]> * Format all the things --------- Co-authored-by: Cool-Katt <[email protected]> Co-authored-by: Derk-Jan Karrenbeld <[email protected]>
1 parent 79d0d12 commit a5bd153

File tree

5 files changed

+42
-21
lines changed

5 files changed

+42
-21
lines changed

.prettierignore

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
/.github/labels.yml
2-
/.github/workflows/sync-labels.yml
3-
/.github/workflows/no-important-files-changed.yml
2+
3+
# Generated
44
exercises/**/README.md
5+
pnpm-lock.yaml
6+
57
!/README.md
68

79
# Originates from https://github.com/exercism/org-wide-files
@@ -19,4 +21,4 @@ config.json
1921

2022
# Originates from https://github.com/exercism/problem-specifications
2123
exercises/practice/**/.docs/instructions.md
22-
exercises/practice/**/.docs/introduction.md
24+
exercises/practice/**/.docs/introduction.md

exercises/concept/appointment-time/.docs/hints.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44

55
- You need to create a new date. The introduction elaborates on the different ways.
66
- `Date.now()` gives you current time in milliseconds
7-
- A day consist of 24 hour. An hour consist of 60 minutes. A minute consist of 60 seconds. A second consist of 1000 milliseconds. In order to get timestamp of `n` days later from current date, you can sum current timestamp and `n * 24 * 60 * 60 * 1000`.
7+
- `Date` has several getter methods, listed in the introduction, to get date components. Can you use one of those methods?
8+
- Likewise, `Date` has matching setter methods to set those components, rolling over into "higher" components if needed.
89

910
## 2. Convert a date into a timestamp
1011

exercises/concept/appointment-time/.meta/config.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
"SalahuddinAhammed",
44
"SleeplessByte"
55
],
6+
"contributors": [
7+
"BadIdeaException"
8+
],
69
"files": {
710
"solution": [
811
"appointment-time.js"

exercises/concept/appointment-time/.meta/exemplar.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99
* @returns {Date} the appointment
1010
*/
1111
export function createAppointment(days, now = Date.now()) {
12-
return new Date(now + days * 24 * 3600 * 1000);
12+
const date = new Date(now);
13+
date.setDate(date.getDate() + days);
14+
15+
return date;
1316
}
1417

1518
/**

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

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,33 +10,45 @@ 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(
34+
new Date(expectedTime),
35+
);
3236
});
3337

34-
test('uses the actual current time when it is not passed in', () => {
35-
const result = createAppointment(0);
38+
test('creates appointment with potential DST change', () => {
39+
const offset = 180; // days
40+
41+
const currentTime = Date.UTC(2000, 6, 1, 12, 0, 0, 0);
42+
let expectedTime = currentTime + offset * 24 * 60 * 60 * 1000;
43+
// Manually adjust for DST timezone offset:
44+
expectedTime +=
45+
(new Date(expectedTime).getTimezoneOffset() -
46+
new Date(currentTime).getTimezoneOffset()) *
47+
60 *
48+
1000;
3649

37-
expect(Math.abs(Date.now() - result.getTime())).toBeLessThanOrEqual(
38-
// Maximum number of time zones difference
39-
27 * 60 * 60 * 1000,
50+
expect(createAppointment(offset, currentTime)).toEqual(
51+
new Date(expectedTime),
4052
);
4153
});
4254

0 commit comments

Comments
 (0)