isaacpitwa/Todo-list

Jest Test Code Review April 1st, 2022

Opened this issue · 0 comments

Highlights

  • Nice use of semantic labeling. Your code is easy to follow given the names of your methods and variables. Super clean!
  • Code is very dry. Good use of .beforeAll() & .beforeEach() to prevent a lot of code repetition
  • JS Best Practice principles used!

Considerations
(NIT) - This is me being picky, feel free to consider this 👍

describe('Add Todo functionality Tests', () => {
beforeAll(() => {
document.body.innerHTML = fs.readFileSync('dist/index.html');
listBeforeAction = document.querySelectorAll('#todos-list li');
desc = document.getElementById('add-todo');
desc.value = 'Test Description';
addTodo();
});
test('Add one new item to the list (View)', () => {
listAfterAction = document.querySelectorAll('#todos-list li');
expect(listAfterAction).toHaveLength((listBeforeAction.length + 1));
});
test('Storage Todos is defined', () => {
expect(localStorage.todos).toBeDefined();
});
test('Storage Todos is not null', () => {
expect(localStorage.todos).not.toBeNull();
});

I still suggest being more specific in your tests. Accessing the actual element and matching the description to your test will shorten your code. It verifies that the storage isn't null AND that it is defined (because both need to be true to access the data). This saves you space in code and is a much more specific test to demonstrate that everything is working as intended.

Lines 12-14. Could set items to null instead of initializing them with an empty value. It is deemed conventional to do that and may make your code more readable to more tenured programmers.

Thanks!