prateekbh / preact-material-components

preact wrapper for "Material Components for the web"
https://material.preactjs.com
MIT License
553 stars 81 forks source link

Better building & testing #1278

Closed cromefire closed 5 years ago

cromefire commented 5 years ago

Improving build:

cromefire commented 5 years ago

@prateekbh Check this out:

yarn run v1.13.0
$ ts-node -P scripts/tsconfig.json scripts/compile.ts
Collecting packages...
Preparing compilation...
Compiling: base
Compiling: icon
Compiling: button
Compiling: card
Compiling: checkbox
Compiling: chip
Compiling: dialog
Compiling: list
Compiling: drawer
Compiling: elevation
Compiling: fab
Compiling: form-field
Compiling: icon-button
Compiling: image-list
Compiling: layout-grid
Compiling: line-ripple
Compiling: linear-progress
Compiling: menu
Compiling: menu-surface
Compiling: radio
Compiling: select
Compiling: slider
Compiling: snackbar
Compiling: switch
Compiling: tab
Compiling: textfield
Compiling: top-app-bar
Compiling: typography
Compiling [============================] 28/28 100%

Done in 29.13s.

tsickle integrated

artipost commented 5 years ago

Failed pictures (50): failed-pictures.zip

:postbox: Posted by artipost.io, through Travis build 488117440, for commit 5ef2fad.

cromefire commented 5 years ago

Well turned out, it didn't actually work...

cromefire commented 5 years ago

which leaves the better feedback point

cromefire commented 5 years ago

@prateekbh I'd like to not only depend on the screenshot testing, but have something like karma unit tests. They also work very well in different browsers (so we can do cross-browser testing) and are far less fragile.

cromefire commented 5 years ago

Also I spoke with the tsickle guys and they meant it's not possible for a library to be compatible to closure without any big restructure

prateekbh commented 5 years ago

Yeah I am not sure Keen on closure, and unit testing is great but let's only restrict it to the parts where we are significant custom code, and not in simpler components like button

cromefire commented 5 years ago

Yes I think closure is not worth the investment.

For the unit tests I hope to be able to replace the documentation testing, because it's very fragile has to be fixed every time

prateekbh commented 5 years ago

Yes. Also I talked to someone who wants to do better visual regression testing for us. Like really nice on screen diffs n all so we can replace current puppeteer with that may be and unit tests for stuff like drawer etc.

cromefire commented 5 years ago

It would really nice if the visual regression testing is cross-browser (or at least Chrome and Firefox) as we could speed up Travis build or test in multiple browsers.

prateekbh commented 5 years ago

Yes that's what I have been told

cromefire commented 5 years ago

Nice, do you know when it can be added?

prateekbh commented 5 years ago

Starting Monday 😊

cromefire commented 5 years ago

@prateekbh Do you have any idea, how to fix the compile script, so that it does not exit, before it's finished?

prateekbh commented 5 years ago

@cromefire i'd need some more details are you referring to this: https://travis-ci.org/prateekbh/preact-material-components/jobs/528169978

^ seems to be completed no?

cromefire commented 5 years ago

Fixed it

prateekbh commented 5 years ago

Ready for review?

cromefire commented 5 years ago

Yes

cromefire commented 5 years ago

You requested smaller PRs so I wanted to put the bind-decorator thing into it's own PR

prateekbh commented 5 years ago

Ok all looks good, just waiting for the tests to pass

cromefire commented 5 years ago

It will not pass because I downvoted them, because the images are already fixed in other PRs. But I Reviewed them and it's okay.

cromefire commented 5 years ago

Also could you please mark 2.0 as protected branch, so I can't push to it by accident?

prateekbh commented 5 years ago

If the tests broke in this PR shouldn't we fix them in the same PR as well?

cromefire commented 5 years ago

They didn't break in this PR, but because applitools works global, there are unmerged PRs changing the global state