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

re-addChore/tests select dataset model #186

Merged
merged 30 commits into from
Sep 29, 2023

Conversation

imda-amdlahir
Copy link
Contributor

@imda-amdlahir imda-amdlahir commented Sep 29, 2023

Description

PR contains mostly unit tests updates for canvas and user input screen (select model , dataset and fill inputs).
Also contains minor improvement to when the saved message is displayed when save button is clicked on header.

  • Most unit tests code changes are under __tests__ and __mock__ subfolders (also include jest files)
  • Code changes under ai-verify-portal/src/modules/project/header.tsx are additions of lines on save button click handling. The existing save function is called and no change is made to that. Additional new codes are added after it to ensure that the Project saved message is displayed only after save is successful. Previously, it was just displayed without checking if save is successful. With this improvement, unit test can assert the APIs that's needed to be called when save is clicked.
  • All other code changes that has additions of toErrorWithMessage simply improves the error message formatting (out put are normally just logged in browser console). No impact to functionality.
  • There is 1-liner code change in ai-verify-portal/src/modules/projectTemplate/canvas.tsx where new Date().getTime() is changed to Date.now(). Both return the exact same thing. Changing to Date.now() allows us to stub and mock it in unit tests`
  • Another addition in ai-verify-portal/src/modules/projectTemplate/canvas.tsx are isNaN handlings which only happens in unit tests jsdom, because there are no screen pixel level calculations or details available.

Motivation and Context

Increase unit tests coverage

Type of Change

  • chore: Routine tasks, maintenance, or tooling changes

How to Test

  1. Under ai-verify-portal root folder, run npm test
  2. On report canvas, ensure that widget drag & drop behaviours are normal
  3. Click save icon on header and see that Project Saved message is displayed

Checklist

Please check all the boxes that apply to this pull request using "x":

  • [ x] I have tested the changes locally and verified that they work as expected.
  • I have added or updated the necessary documentation (README, API docs, etc.).
  • [x ] I have added appropriate unit tests or functional tests for the changes made.
  • [x ] I have followed the project's coding conventions and style guidelines.
  • I have rebased my branch onto the latest commit of the main branch.
  • [x ] I have squashed or reorganized my commits into logical units.
  • I have added any necessary dependencies or packages to the project's build configuration.
  • [x ] I have performed a self-review of my own code.
  • [x ] I have read, understood and agree to the Developer Certificate of Origin below, which this project utilises.

Screenshots (if applicable)

image

Additional Notes

[Add any additional information or context that might be relevant to reviewers.]

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
   have the right to submit it under the open source license
   indicated in the file; or

(b) The contribution is based upon previous work that, to the best
   of my knowledge, is covered under an appropriate open source
   license and I have the right under that license to submit that
   work with modifications, whether created in whole or in part
   by me, under the same open source license (unless I am
   permitted to submit under a different license), as indicated
   in the file; or

(c) The contribution was provided directly to me by some other
   person who certified (a), (b) or (c) and I have not modified
   it.

(d) I understand and agree that this project and the contribution
   are public and that a record of the contribution (including all
   personal information I submit with it, including my sign-off) is
   maintained indefinitely and may be redistributed consistent with
   this project or the open source license(s) involved.

kimeetok
kimeetok previously approved these changes Sep 29, 2023
Copy link
Contributor

@imda-benedictlee imda-benedictlee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

… which are already defined in package.json and remove ts-jest
@imda-amdlahir
Copy link
Contributor Author

@imda-benedictlee
All 68 unit tests pass. Status is red because of code coverage threshold.
Also pushed an npm audit fix and updated workflow yaml to omit dev dependencies in npm audit flow

Copy link
Contributor

@imda-benedictlee imda-benedictlee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested. Working as expected. Screenshot attached.

Project Saved
image

Project Revisited
image

@imda-benedictlee imda-benedictlee merged commit 1571263 into main Sep 29, 2023
4 of 6 checks passed
@imda-benedictlee imda-benedictlee deleted the chore/tests-select-dataset-model branch September 29, 2023 08:38
iamksuresh pushed a commit that referenced this pull request Nov 13, 2024
re-addChore/tests select dataset model

The requested reviews are Approved. Proceeded to merge PR to Main.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend-portal frontend portal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants