Codecov Report

Merging #4319 (c637a86) into master (7194b88) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4319   +/-   ##
=======================================
  Coverage   98.70%   98.70%           
=======================================
  Files         205      205           
  Lines        7320     7320           
  Branches     2083     2083           
=======================================
  Hits         7225     7225           
  Misses         35       35           
  Partials       60       60           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7194b88...c637a86. Read the comment docs.

#4318 replaces fs.writeFileSync and sander.writeFileSync with atomicWriteFileSync for reasons described in the link.

In an experiment, replacing await sander.copydir() with an equivalent execSync call copying the directory via shell commands did not improve the reliability of certain node12/macos watch tests. Curiously, I haven't seen these watch test failures in node16/macos. Not sure if this is due to node16 being a faster JIT VM, or whether a NodeJS bug on macos was fixed after node12, or if there is some other issue with the tests themselves, chokidar, or rollup.

Curiously, I haven't seen these watch test failures in node16/macos.

I spoke too soon - here's a rare node16/macos failure in the commits of this PR:

https://github.com/rollup/rollup/runs/4634674788?check_suite_focus=true

In an experiment, replacing await sander.copydir() with an equivalent execSync call copying the directory via shell commands did not improve the reliability of certain node12/macos watch tests.

yeah, I didn't think the watch failures would go away with this. I agree with you that the timing and execution of the tests is likely at fault. I wanted to look after the last commits at the failures and/or wait for the other PR first.

I went a little further and made this PR also fully async.

I like the idea of getting rid of sander, but maybe you are trying too much here by "fixing everything". Why not limit this one to replacing sander for now but keep especially the structure of the watch tests intact, and then improve them separately?

I like the idea of getting rid of sander, but maybe you are trying too much here by "fixing everything". Why not limit this one to replacing sander for now but keep especially the structure of the watch tests intact, and then improve them separately?

good point. I haven't looked at this PR for a while because of the mac os ci failures.
let me rebase and revert some stuff to make it stable.

@lukastaegert this should be good to go