Replace fireworm with sane #1482
Conversation
|
@johanneswuerbach do you think you might have time to look into that if it is good to be merged? :) |
|
Hey sorry, I've retriggered CI, which would need to be green before merging this (except the browser-tests jobs). |
|
Hmm, interesting. I am not really sure what the issue is there - it seems to be some kind of weird (?) timing issue. Not 100% sure if it is actully an introduce "failure" or some other thing. When trying to debug it, I for example increased the timeout here: which also lead to the test failing on master as well (e.g. set it to 1000, and tests will fail). Which, if I understand the test correctly, shouldn't be the case? So I could it be it actually just shows some fundamental timing issue with the test here, maybe? But not 100% sure how all of that plays together, to be honest...! |
mydea
force-pushed
the
fn/replace-fireworm-sane
branch
from
a113ba4
to
def0c06
Compare
|
anything that I can do to help move this along? |
|
@NullVoxPopuli and others, I forked @mydea’s branch, tweaked the GitHub Actions configuration a bit, and I was able to surface the following errors that we have to look into: https://github.com/gnclmorais/testem/runs/5611283650 |
|
Pinging the thread in case a testem contributor has time to look at the PR 😁 |
|
This PR (or a new version of this) would need to be updated and tests passing, afterwards I'm happy to merge this. |
This PR replaces the pretty old fireworm dependency with sane.
The actual watching behavior was not really tested that well, so I tried to add some more tests to cover if it actually picks up changing files, and if the core configuration works as expected. I think the watching used to ignore the
cwdsetting (if I understood everything correctly), which it now shouldn't.To test the file watching, I had to add a timeout to the test.. doesn't feel super nice, but I wasn't sure how else to properly test this. Happy to change this if there is a better way to do this!
Closes #1481