London | 26-ITP-Sep | Sakiya Mayow | Sprint 1 | Structuring and Testing Data

[zakiaao-tech]

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

In this task I added an if statement because when testing the code I noticed that for any hours < 10 is a single digit number but we need double digits and added 0 where it was supposed to go. I then tested a few edge cases and fixed the function according to the mistakes and failed tests.

[cjyuan]

Your test misses several cases.

You should update your test first before revising your function implementation.

[cjyuan]

It seem your tests only cover two cases:

• can correctly convert morning time
• correctly convert time after 12:00

What about the boundary cases?

Could the function correctly handle the minute in the time? For examples, 00:34, 23:59?

[abdishakoor-dev]

23:59 is tested now. 00:34 isn't yet; it's in the list in step 1 of my overall comment.

[abdishakoor-dev]

Writing the tests first and then adding minutes to the function, as your last two commits show, was the right order. 23:59 and 12:01 both come out right now.

There's one bug left in the function, and a few tests to add that will show it to you. Here are the steps:

1. Add a test for each of these times: "11:59", "00:00", "00:34", "12:00". Write each test in the same shape as yours. The first "..." is the time going in. The second "..." is what your function should give back, as a 12-hour clock would show it. Decide that yourself before you run anything, for example: what does a 12-hour clock show at 00:34?

   test("describe the time you are checking", function () {
     assert.equal(formatAs12HourClock("..."), "...");
   });

2. Run the tests. Right click the format-clock-edge-cases folder in VS Code, choose Open in Integrated Terminal, and run node --test. The 11:59 test should fail and show you what your function really returns: 11:59: 59 am. That's good. It means the test has found a bug, and every time from 10:00 to 11:59 has it.
3. Fix the bug. It's on timeConverter.js line 24 (see my comment there). Run node --test again until every test passes.
4. Sort out the four tests on lines 13 to 27. See my comment on line 13.

When all of that is pushed and every test passes, it should be ready to mark Complete. Add the Needs Review label again once you've pushed.

[abdishakoor-dev]

time holds the whole input, for example "10:30". So ${time} already includes the minutes, and : ${minutes} then adds them a second time, with a space in front. That's how you get 10:30: 30 am.

For 10:00 to 11:59 the hours already have two digits, so nothing needs changing about them. What does ${time} on its own give you, and is that all you need before am?

[abdishakoor-dev]

Bugs often hide at the exact point where something changes. 12:01 is the first minute after noon, so the last minute before noon, 11:59, is just as important. Your function gets 11:59 wrong at the moment, and a test for it would have shown you. Midnight (00:00) and noon (12:00) are the other two points where the clock changes, so they need a test each too.

[abdishakoor-dev]

These four tests all have the same name, "correctly convert time after 12:00". If one of them failed, the output wouldn't tell you which time broke. They also all check a whole hour between 2pm and 9pm, so they all test the same thing.

Keep one of them, and give it a name that says the time it checks, for example "converts 14:00 to 02:00 pm". Delete the other three. Your new tests from step 1 of my overall comment test far more.