Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Update the existing test cases and include new test cases for every component #174

Open
1 of 2 tasks
KC900201 opened this issue Nov 6, 2024 · 9 comments
Open
1 of 2 tasks
Assignees

Comments

@KC900201
Copy link
Contributor

KC900201 commented Nov 6, 2024

Description:

I will update some existing test cases for the components in the project and include new unit test scripts for other components. Below is a summary of the implementation covered in this issue:

  • Include unit test and functional test scripts for testing component files and functional scripts
  • Include test coverage for each test

Expected outcome

  1. For standard coding practice and maintenance, it's recommended to write out unit tests for every new feature introduced in future
  2. The existing test scripts can serve as a template for point (1)
@KC900201
Copy link
Contributor Author

KC900201 commented Nov 6, 2024

cc: @RayMathew, @Buzzpy

Would like to have your opinion on this feature

@RayMathew
Copy link
Contributor

@KC900201

  1. I'm a little confused when you say test 'scripts'. Do you just mean additional unit test files?
  2. Adding unit tests is always a good thing. No objections there.
  3. Agreed that every feature should come with unit tests. Or at least, unit tests should be mandatory in places where they would have a significant impact.
  4. I don't agree with moving all test files to a separate tests folder. Unit tests are meant to be granular, testing the smallest of logic and interactions. And they need to be updated when you're updating the feature. It only naturally follows that the unit test file should be right next to the file it is testing.

@KC900201
Copy link
Contributor Author

KC900201 commented Nov 7, 2024

@RayMathew

Thanks for your opinion. Yes, I mean test files

@KC900201
Copy link
Contributor Author

KC900201 commented Nov 7, 2024

Kindly assign to me if this is workable

@KC900201
Copy link
Contributor Author

Added PR-175

@KC900201
Copy link
Contributor Author

@RayMathew, @Buzzpy.

I'd like some guidance on writing the test file for the Modal.astro component. I noticed that the component is linked to the function script modal.ts which has in eventListener function. In order to emulate the event listener and write the assertion test, I'd need to write a mock function that behaves like modal.ts.

Do you have any ideas? Below is the skeleton code for the test script

import { experimental_AstroContainer as AstroContainer } from "astro/container"
import { expect, test } from "vitest"
import Modal from "./Modal.astro"

describe("test Modal component", () => {
  let container: AstroContainer

  beforeEach(async () => {
    // instantiate Astro container api
    container = await AstroContainer.create()
  })

  test("user click one of the terms card and the explanation modal is explained", async () => {
    // To-do: mock function for mocking modal.ts

  })
})

@RayMathew
Copy link
Contributor

RayMathew commented Nov 17, 2024

Take what I'm saying with caution, since I haven't looked at Astro testing so far and the best practices.

Going through the code, it doesn't make sense to me to test Modal.astro, since there's almost nothing in it.

It should be the reverse. Test modal.ts, and mock the contents of Modal.astro. Like this:

test('should display modal with given content', () => {
  // Create a mock modal element in the DOM
  document.body.innerHTML = `
    <div id="modal" style="display: none;">
      <div id="modal-content">
        <div id="modal-body"></div>
      </div>
    </div>
  `;
  
  showModal("Test Title", "This is a test description.", "https://example.com/image.jpg", "https://example.com/resource");

  const modal = document.getElementById("modal");
  const modalBody = document.getElementById("modal-body");

  expect(modal?.style.display).toBe("block");
  expect(modalBody?.innerHTML).toContain("Test Title");
  expect(modalBody?.innerHTML).toContain("This is a test description.");
  expect(modalBody?.innerHTML).toContain('<img src="https://example.com/image.jpg"');
  expect(modalBody?.innerHTML).toContain('<a href="https://example.com/resource"');
});

I realise that ideally we should be testing .astro files since this is an Astro project. But since the migration hasn't been done you have a choice. 1) Migrate the code from modal.ts to Modal.astro and then test. 2) Test modal.ts in its current state and leave migration for a future task.

@KC900201
Copy link
Contributor Author

KC900201 commented Nov 17, 2024

Thanks, @RayMathew for the advice. I'll update the test file right away. For the moment, I agree that 2.) would be a better option for the moment.

@KC900201
Copy link
Contributor Author

KC900201 commented Nov 17, 2024

Hi @Buzzpy,
cc: @RayMathew

PR-175 is updated with test modal.ts for the functional test. Notice that only positive assertion tests are included for the moment. Kindly comment if negative assertion tests are also required. Otherwise, the PR should be ready to merge.

Update: any comments? @Buzzpy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants