London | 26-ITP-Sept | Hugh Mills | Sprint 1 | formatAs12HourClock by HM-127BTY · Pull Request #1636 · CodeYourFuture/Module-Structuring-and-Testing-Data

HM-127BTY · 2026-10-03T11:08:44Z

Learners, PR Template

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

Changelist

Done tests and made adjustments for extra test requests for the code.

Added in time with minutes for am and pm

abdishakoor-dev

Good work getting noon right: 12:00 and 12:30 now come out as pm. The minutes after midnight also stay in place now, and every pm time keeps the same two-digit shape as the mornings.

There are four things to do before I can mark this Complete. I've put a hint for each one in the inline comments, so work through them in this order:

Fix midnight. formatAs12HourClock("00:00") returns "00:00 am", but a 12-hour clock never shows 00. It should be "12:00 am". See my comment on timeConverter.js line 11.
Add the missing tests. The task asks you to test the edge cases: the times where your function starts doing something different. My comment on timeConverter.test.js line 23 lists the times to test and the answer each one should give.
Delete the commented-out code on timeConverter.js lines 6 to 10.
Format both files with Prettier. "My code is consistently formatted" is on the checklist, and the tool that does it for you is called Prettier. It rearranges spacing and indentation to one agreed style, so that your code is easy to read and so that a reviewer only sees the changes you meant to make, not stray spaces and tabs. At the moment both of your files fail that check.

Prettier comes with the CYF extension pack you were asked to install during onboarding. If you're not sure you have it, open VS Code, go to Extensions, and search for CodeYourFuture Extension Pack; install it if it isn't there: https://marketplace.visualstudio.com/items?itemName=CodeYourFuture.cyf-extension-pack

Then open each of your files, right click in the editor, choose Format Document, and pick Prettier if VS Code asks which formatter to use. Save, commit the changes it makes and push. To make this happen automatically every time you save, follow the format on save steps here: https://github.com/CodeYourFuture/Module-JavaScript-Fundamentals/blob/main/practical_guide.md

To check your work, run node --test format-clock-edge-cases/*.test.js from the repo folder. Every test should show a green tick.

If you get stuck on any of these, reply on the comment and I'll help. Add the Needs Review label again once you've pushed and I'll take another look.

abdishakoor-dev · 2026-10-04T08:48:20Z

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


You've handled the 12 hour here. The other special hour is 00, the first hour after midnight.

At the moment formatAs12HourClock("00:00") falls through to the last line and returns "00:00 am". On a 12-hour clock midnight is written as 12:00 am, and half past midnight as 12:30 am.

Hint: add another check like the one on this line, but for hours === 0. Inside it, return a string that starts with 12: and ends with am. You already have the minutes in min.

When it works you should get:

formatAs12HourClock("00:00") gives "12:00 am"

formatAs12HourClock("00:30") gives "12:30 am"

formatAs12HourClock("01:00") still gives "01:00 am"

Ah I understand, I have done the changes for this so it should be seen now in the code.

That's it. 00:00 and 00:30 both come out right now. Well done, this was the trickiest bit.

abdishakoor-dev · 2026-10-04T08:48:20Z

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


Please delete lines 6 to 10. Old code should be removed rather than commented out. Git keeps every version you commit, so you can always get it back.

Understood, I recall you telling me before, think I forgot to remove it at the time. ^^

Gone now. Good.

abdishakoor-dev · 2026-10-04T08:48:20Z

 function formatAs12HourClock(time) {

  const hours = Number(time.slice(0, 2));
+  const min = Number(time.slice(-2));


Optional, not needed for Complete: min is turned into a number here, and then on line 14 it is turned back into a string and padded with a 0. If you kept it as a string, time.slice(-2) already gives you "04" with the 0 in place. Then you wouldn't need .toString().padStart(...) for the minutes.

I had it showing up as "4" a few times when I was doing a test so I just put that in to ensure it was working, might have messed up something on my end to make it do that.

That makes sense: Number("04") gives 4, which is why you needed the padStart. It works as it is, so leave it.

abdishakoor-dev · 2026-10-04T08:48:20Z

+    assert.equal(formatAs12HourClock("18:35"), "06:35 pm");
+});
+
+test("can correctly convert time with minutes with 0 pad such as 12:04", function(){


The test name says 12:04 but the test checks 13:04. Make the name match what is tested. For example "keeps the leading 0 in the minutes, such as 13:04".

One small one still open here: the name on line 22 says 12:04, but line 23 checks 13:04. Change it to:

test("can correctly convert time with minutes with 0 pad such as 13:04", function () {

That's the only change on that line.

I see my error, I put in an example and it didn't match up with the code. Fixed the error there.

Matches now.

abdishakoor-dev · 2026-10-04T08:48:20Z

+
+test("can correctly convert time with minutes with 0 pad such as 12:04", function(){
+    assert.equal(formatAs12HourClock("13:04"), "01:04 pm");
+});


Edge cases are the times where the answer changes, plus the times just before and just after. For a clock, those are midnight, noon and the very end of the day. Please add a test for each of these, using the same shape as your other tests:

Input Expected

"00:00" "12:00 am"

"00:01" "12:01 am"

"11:59" "11:59 am"

"12:00" "12:00 pm"

"12:01" "12:01 pm"

"13:00" "01:00 pm"

"23:59" "11:59 pm"

Give each test a name that says why it matters, for example test("midnight is 12 am", ...) or test("one minute before noon is still am", ...).

Write the two 00: tests before you fix line 11 of timeConverter.js. Run them and you'll see them fail. After your fix they should pass. That's the point of the tests: they catch the bug for you.

Added in the tests for 00:00 and 00:01 and got it working.

Your two 00: tests are right, nice work. Now the same for these five:

Input Expected

"11:59" "11:59 am"

"12:00" "12:00 pm"

"12:01" "12:01 pm"

"13:00" "01:00 pm"

"23:59" "11:59 pm"

Here is the first one written out, so you can see the shape:

test("noon is 12 pm", function () { assert.equal(formatAs12HourClock("12:00"), "12:00 pm"); });

Steps:

Go to the very end of timeConverter.test.js, after line 33, and leave one empty line.

Paste the test above.

Copy it four more times. In each copy change the input, the expected answer and the name, using the table. Names that say why the time matters work well, for example "one minute before noon is still am", "one minute after noon is pm", "1 pm shows as 01:00 pm" and "the last minute of the day is 11:59 pm".

Save (Prettier will tidy the spacing), then run the tests.

Your code should already pass all five. Why test them anyway? Noon was one of the bugs you fixed, and right now there is no test for it, so if someone broke it later nothing would tell them. These tests are what would catch it.

Ok, added in the tests there and worded them aswell.

All five in and passing, with names that say why each time matters. Good.

abdishakoor-dev

Really good progress this round. Midnight was the hardest part of this task, and your fix works: I tried every time from 00:00 to 23:59 and your function got all of them right. The commented-out code is gone and both files pass Prettier too. Thanks for replying on each comment as well, that makes reviewing much easier.

You're nearly there. Two small things left, both in timeConverter.test.js, and neither needs any change to your function:

Add five more tests: 11:59, 12:00, 12:01, 13:00 and 23:59. I've written out the first one for you in my reply on line 23, so you can copy it and change the time for the other four.
Line 22: change 12:04 in the test name to 13:04, so the name matches what line 23 checks.

Then run node --test format-clock-edge-cases/*.test.js from the repo folder. You should see 12 green ticks. If any test fails or you get stuck, reply here and tell me what you see, and I'll help.

Add the Needs Review label again once you've pushed. After that I expect to mark this Complete.

abdishakoor-dev

All five tests are in and all 12 pass, and the test name on line 22 matches its input now. Your tests now guard every bug you fixed, including noon and midnight. That's everything, marking this Complete. Well done for sticking with it.

HM-127BTY added 2 commits October 1, 2026 00:49

Testing and adding in code:

e5c8086

Added in time with minutes for am and pm

added test for ensuring pad of 0 for single digit min

1f0220a

This comment has been minimized.

Sign in to view

HM-127BTY added 📅 Sprint 1 Assigned during Sprint 1 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Oct 3, 2026

abdishakoor-dev reviewed Oct 4, 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 4, 2026

HM-127BTY added 4 commits October 5, 2026 14:48

deleted commented out code from lines 6-10

f85f166

Added in special hour of 00 to testing and adjust code.

77d1ade

Added extra test for 12:01 am

158eba2

Ensured extensions are added correctly and formatted document

cf1ca72

HM-127BTY 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 removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 5, 2026

HM-127BTY added 2 commits October 6, 2026 02:28

Added 5 more tests as requested.

b56500c

fixed example in test to match the test itself.

2fc03a9

HM-127BTY added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Oct 6, 2026

abdishakoor-dev reviewed Oct 6, 2026

View reviewed changes

Input	Expected
`"00:00"`	`"12:00 am"`
`"00:01"`	`"12:01 am"`
`"11:59"`	`"11:59 am"`
`"12:00"`	`"12:00 pm"`
`"12:01"`	`"12:01 pm"`
`"13:00"`	`"01:00 pm"`
`"23:59"`	`"11:59 pm"`

Conversation

HM-127BTY commented Oct 3, 2026

Learners, PR Template

Task code

Changelist

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!

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!

abdishakoor-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants