London | 26-ITP-September |Abdennour Hachemi | Sprint 1 | Structuring and testing data : formatAs12HourClock by AbdennourHachemi · Pull Request #1647 · CodeYourFuture/Module-Structuring-and-Testing-Data

AbdennourHachemi · 2026-10-05T12:04:44Z

Self checklist

I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide

Task code

CYF-1197

abdishakoor-dev

Good work on the afternoon times. "23:59" now gives "11:59 pm" and "13:00" gives "01:00 pm", which fixes the biggest bug in the starter. You also kept the two starter tests as they were, and both files are formatted.

There are two hours of the day left to fix, plus some tests to add. Here are the steps:

Noon to 1pm. Try "12:30". Is half past twelve in the afternoon am or pm, and what does your function say? See my comment on timeConverter.js line 32.
Midnight to 1am. Try "00:30". A 12-hour clock has no hour 00, so what should it show? Your function only handles exactly "00:00". See my comments on timeConverter.js line 22 and timeConverter.test.js line 20.
Add a test for each of these times: "00:01", "11:59", "12:01", "13:00", "23:59". Each one sits on the edge of a part of the day. Work out the expected 12-hour time for each before you write the test. Each test has the same shape as the ones you already wrote:

test("describe the case here", function () {
  assert.equal(formatAs12HourClock("..."), "...");
});

Run node --test from inside the format-clock-edge-cases folder after adding each one. A failing test means you've found a bug: fix the code until it passes.

Remove the two console.log lines in timeConverter.js (lines 2 and 5).

Once those four are pushed and all the tests pass, I expect to mark this Complete. Add the Needs Review label again when you've pushed.

abdishakoor-dev · 2026-10-05T15:37:53Z

+  if (hours < 12) {
+    return `${time} am`;
+  } else if (hours == 12) {
+    return `${time} am`;


This branch handles every time from 12:00 to 12:59. Line 26 returns pm for "12:00", but this line returns am for "12:30". Which one is right for the rest of that hour?

12:30 gives pm now. Good.

abdishakoor-dev · 2026-10-05T15:37:53Z


-  if (hours > 12) {
-    return `${hours - 12}:00 pm`;
+  if (time === "00:00") {


This compares the whole time to the text "00:00", so "00:01" to "00:59" don't match and fall through to line 30. Which part of the time tells you it's the midnight hour? You already have it in a variable on line 19. What should replace the 00 for every minute of that hour, and how can you keep the minutes the user gave you?

Deleting this branch means "00:00" breaks as well now. The idea from my first comment still works: check hours (line 17) instead of the whole text, and build the answer from 12:, the minutes and am. Careful with line 18: it turns the minutes into a number, so 5 minutes would come out as 5, not 05. Which part of time already holds the minutes as two characters? Also think about where the new check goes: line 24 catches every hour below 12, and that includes hour 0.

Fixed. Every minute from 00:00 to 00:59 gives 12:.. am now.

abdishakoor-dev · 2026-10-05T15:37:53Z

+  assert.equal(formatAs12HourClock("12:00"), "12:00 pm"));
+
+test("can corectly convert half an hour passed midnight", () =>
+  assert.equal(formatAs12HourClock("00:30"), "00:30 am"));


Look at what you expect for midnight on line 14. Does a 12-hour clock show 00 half an hour later? Update the expected value here. This test should fail until timeConverter.js line 22 is fixed.

Right value now.

abdishakoor-dev · 2026-10-05T15:37:53Z

@@ -1,11 +1,51 @@
 function formatAs12HourClock(time) {
+  console.log(time.length);


These console.log lines (here and line 5) were useful while you were working it out, but they print every time the tests run. Delete both lines.

Both gone, but one left a c; behind on line 3. See my new comment there.

abdishakoor-dev

The noon hour is fixed: "12:30" gives "12:30 pm" now. Your test on line 20 has the right value and the console.log lines are gone.

Some of the other changes broke things that worked before, though. Run node --test inside the format-clock-edge-cases folder and you'll see 4 of your 7 tests fail, including the starter's "23:00" test. Use those failures as your to-do list:

"9:00" crashes. See my comment on timeConverter.js line 3.
Afternoon times keep their 24-hour number: "23:00" gives "23:00 pm". See my comment on line 27.
Midnight is wrong again: "00:00" and "00:30" give 00. See my reply on the line 22 thread.
The five tests from last time ("00:01", "11:59", "12:01", "13:00", "23:59") still need adding.

Fix one thing at a time and run node --test after each change. Only push when every test passes. Then add the Needs Review label again. Once all the tests pass, I expect to mark this Complete.

abdishakoor-dev · 2026-10-06T10:50:25Z

@@ -1,11 +1,55 @@
 function formatAs12HourClock(time) {
+  if (time.length == 4) {
+    c;


This c; looks left over from deleting the console.log. JavaScript doesn't know what c is, so any time with a one-digit hour, like "9:00", stops here with ReferenceError: c is not defined. Delete this line and that test passes again.

Gone, and "9:00" passes again.

abdishakoor-dev · 2026-10-06T10:50:25Z

+  if (hours < 12) {
+    return `${time} am`;
+  }
+  if (hours >= 12) {


This if catches every hour from 12 to 23, so the afternoon branches from line 34 down never run any more. They were giving the right answers before this push. Only one hour of the day should keep its number and just get pm added. Which hour is that? Change the condition so it only catches that hour.

Fixed. "23:00" gives "11:00 pm" again.

abdishakoor-dev

Every time of the day is right now. I checked all 1,440 minutes from 00:00 to 23:59, and your function gets each one. The c; is gone and the afternoon works again.

Two things left, and then this is Complete:

Add the five tests from my first review. The inputs are "00:01", "11:59", "12:01", "13:00" and "23:59". For each one, work out the 12-hour time it should give, then write a test the same shape as your midnight test on line 13:

test("describe the case here", () =>
  assert.equal(formatAs12HourClock("..."), "..."));

The first "..." is the input, the second is what the function should give back. Run node --test inside format-clock-edge-cases after each one. They should all pass, because your function is right now. They are there so that if someone changes the code later and breaks one of these edges, a test fails and tells them.

Format timeConverter.js. Lines 2 to 4 and most lines from 11 onwards are indented with three spaces instead of two. Right click in the editor, choose Format Document, and save. To make VS Code do this every time you save, follow the format on save steps here: https://github.com/CodeYourFuture/Module-JavaScript-Fundamentals/blob/main/practical_guide.md

Once both are pushed, add the Needs Review label again and I'll mark it Complete.

abdishakoor-dev

Both files are formatted now, and four of your five new tests are exactly right. One small slip left, then this is Complete:

The "one minute after midday" test checks the wrong time (timeConverter.test.js line 30). See my comment there.

Add the Needs Review label again when you've pushed, and I'll mark it Complete.

abdishakoor-dev · 2026-10-07T08:49:25Z

+  assert.equal(formatAs12HourClock("11:59"), "11:59 am");
+});
+test("can correctly handel one minute after miday", function () {
+  assert.equal(formatAs12HourClock("00:01"), "12:01 am");


This test is called "one minute after midday", but it checks "00:01", the same time as your test on line 24. It looks like a copy and paste slip.

What is one minute after midday on a 24-hour clock, and what should the function give for it? Change both values on this line, then run node --test to check it passes.

The duplicate is gone, but "12:01" still needs its own test. See the review comment for the exact test to add.

abdishakoor-dev

Nearly there. You removed the copy of the "00:01" test, but the minute after midday still has no test, and that's the time it was meant to check. Your first version of the code, "12:30" came out as "12:30 am", and no test caught it. So this is an important test to have in place.

One thing left. Add this test at the bottom of timeConverter.test.js, after the "one minute before midnight" test:

test("can correctly handle one minute after midday", function () {
  assert.equal(formatAs12HourClock("12:01"), "12:01 pm");
});

Then run node --test timeConverter.test.js in the terminal, inside the format-clock-edge-cases folder. It should say pass 12 and fail 0. Push, add the Needs Review label again, and I'll mark this Complete.

AbdennourHachemi added 3 commits October 5, 2026 12:00

solving time convert exercsie

428849e

remove comment

d81fa17

update code

556f283

This comment has been minimized.

Sign in to view

AbdennourHachemi added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 5, 2026

abdishakoor-dev reviewed Oct 5, 2026

View reviewed changes

abdishakoor-dev added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 5, 2026

fixing errors

3e6d6c8

AbdennourHachemi added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 5, 2026

abdishakoor-dev reviewed Oct 6, 2026

View reviewed changes

abdishakoor-dev added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 6, 2026

correcting Errors and reorgnising code

fc0d336

AbdennourHachemi added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 6, 2026

abdishakoor-dev reviewed Oct 6, 2026

View reviewed changes

abdishakoor-dev added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 6, 2026

AbdennourHachemi added 2 commits October 6, 2026 21:44

add more test cases

c97324b

correct indentation

eb2a847

AbdennourHachemi added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 6, 2026

abdishakoor-dev reviewed Oct 7, 2026

View reviewed changes

abdishakoor-dev added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 7, 2026

remove duplicate test case

738f3b7

AbdennourHachemi removed the Reviewed Volunteer to add when completing a review with trainee action still to take. label Oct 7, 2026

AbdennourHachemi added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 7, 2026

abdishakoor-dev reviewed Oct 7, 2026

View reviewed changes

abdishakoor-dev added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 7, 2026

re-instating test for one minute after midday

dfa00be

AbdennourHachemi added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Oct 7, 2026

		@@ -1,11 +1,51 @@
		function formatAs12HourClock(time) {
		console.log(time.length);

Conversation

AbdennourHachemi commented Oct 5, 2026

Task code

Uh oh!

This comment has been minimized.

abdishakoor-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abdishakoor-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abdishakoor-dev left a comment

Choose a reason for hiding this comment

Uh oh!

abdishakoor-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abdishakoor-dev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abdishakoor-dev left a comment •

edited

Loading