llaske / sugarizer

Sugarizer is a web implementation of the Sugar platform to run on any device or browser
https://sugarizer.org
Apache License 2.0
197 stars 412 forks source link

fix: Update tests to accommodate changes in v2 and test utils API #1526

Closed UtkarshSiddhpura closed 6 months ago

llaske commented 7 months ago

Nice start but there is still a failed test.

image

UtkarshSiddhpura commented 7 months ago

Nice start but there is still a failed test.

All tests are passing on my side can you locate which test is failing? It's probably 1 of the four.

llaske commented 7 months ago

It's in password component

image

UtkarshSiddhpura commented 7 months ago

Nice start but there is still a failed test.

Resolved

llaske commented 7 months ago

That's nice but is it possible to remove all console.warn?

image

UtkarshSiddhpura commented 7 months ago

I guess all the warnings are related to props datatypes (for e.g icon x , y accepts Number but all the implemented-screens/components-using-icon are passing it a string) So should i change the prop data-type or everywhere it's passing it as string?

llaske commented 7 months ago

@UtkarshSiddhpura I think that it's okay for x, y or color to be a number but there is no reason to force id to be a number.

UtkarshSiddhpura commented 7 months ago

@llaske I think we should setup something so before pushing or before merging we run all the test cases to avoid potential errors and warnings

llaske commented 7 months ago

Not sure to understand. What do you want to setup? What prevents to fix warnings?

UtkarshSiddhpura commented 6 months ago

Not sure to understand. What do you want to setup? What prevents to fix warnings?

Something like a pre-push hook that only pushes if all the test are passed @llaske You can merge this PR for now I'll open another PR if we're interested in pre-push hook

llaske commented 6 months ago

Good job. Thanks.

Something like a pre-push hook that only pushes if all the test are passed

Got it. You mean action to launch unit testing before commit? I think it's more something to put in Github actions (like for Sugarizer Server here).