London | 26-ITP-Sep | Yonatan Teklemariam | Sprint 1 | Format clock edge cases

[Yonatanteklemariam]

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

Completed the assignment.

Questions

[abdishakoor-dev]

Your function gets the hours and minutes right for every valid time, including midnight, noon and 23:59. Picking 11:59 and 23:59 as test cases shows you were looking for the edges.

A few things before I can mark this Complete:

1. The starter tests have changed. They expected "08:00 am" and "11:00 pm", in lowercase, and those two tests need to stay exactly as written. See my comment on timeConverter.test.js line 6.
2. Your tests can't fail at the moment. See my comment on timeConverter.test.js line 18.
3. One more boundary: noon itself, "12:00". See my comment on line 9.
4. timeConverter.js isn't formatted. "My code is consistently formatted" is on the checklist, and the tool that does it for you is called Prettier. It comes with the CYF extension pack you installed during onboarding (if not: https://marketplace.visualstudio.com/items?itemName=CodeYourFuture.cyf-extension-pack). Open the file, right click, choose Format Document, and pick Prettier if VS Code asks. To make it happen every time you save, follow the format on save steps here: https://github.com/CodeYourFuture/Module-JavaScript-Fundamentals/blob/main/practical_guide.md

Add the Needs Review label again once you've pushed.

[abdishakoor-dev]

The starter tests said "08:00 am" and "11:00 pm", in lowercase. They describe what the function must return, so they stay as written, and the function has to match them, not the other way round. Please put the two starter tests back exactly as they were on main. Then run them: they'll fail, which tells you what to change in timeConverter.js.

[Yonatanteklemariam]

I have also reverted the tests.

[abdishakoor-dev]

The inputs and expected values match the starter again, thanks. For next time, keep the given tests' names as well, so a reviewer can spot the original tests at a glance.

[abdishakoor-dev]

Try this: change line 15 of timeConverter.js to return "broken", then run node --test in this folder. You'll see a ❌ printed, but at the bottom it still says pass 1 and fail 0. Printing a message tells you about a problem, but it doesn't make a test fail. What do the assert and test imports on lines 2 and 3 do, and how did the starter tests use them?

[Yonatanteklemariam]

Thanks for your constructive feedback. To answer your question, test() is Node’s built‑in test runner that tells Node it is a real test, while assert.equal() is the error-checking engine that throws an error when values don’t match. So the starter tests tell node that these are the actual tests and assert.equal() fails when it finds an error; Node sees the error and marks the test as failed.

[abdishakoor-dev]

That's exactly it. Your tests use test and assert.equal now, so a wrong answer really fails.

[abdishakoor-dev]

"12:10" tests a time inside the noon hour, which is good. What about noon exactly, "12:00"? It's the point where am turns into pm, so it's worth its own test, the same way you tested "00:00".

[Yonatanteklemariam]

I have added the test case for noon as suggested.

[abdishakoor-dev]

Good, noon has its own test now.

[abdishakoor-dev]

Everything from my last review is done, and your tests can fail properly now. Marking this Complete. Well done.