digidem / mapeo-core

Library for creating custom geo data and syncronizing via a peer to peer network
23 stars 2 forks source link

Failing windows test on v8 #101

Closed okdistribute closed 4 years ago

okdistribute commented 4 years ago

https://travis-ci.org/github/digidem/mapeo-core/jobs/709302622

It seems to be that there is one less progress event than we expect to happen, but this only shows up on the travis Windows instance.

I opened mapeo-core in my windows machine locally on node 12 and could not reproduce the failing test. This makes it really difficult and time consuming to test because we then have to test it on travis only. Because we can't reproduce this issue in real scenarios, I suspect that it is an oddity that won't affect users at all. Even if it did show up in production, the issue is that there is one fewer progress event than it should have, which is not a breaking change or even that noticable for users. The syncronization still ends as intended, it just has one fewer progress event.

This is what has led me to not prioritize fixing this failing test and instead focus on other bugs with our limited time and release schedule.

Is the test too precise? Should we make the test more forgiving of this race condition? Do we still see the error on Github Actions in their windows environment? Is it travis-only?

hackergrrl commented 4 years ago

I agree! Counting events doesn't really add anything. I think it'd be better to check things like "does the progress ever hit 100% before the final event?", etc.

I just pushed a2dec19200a7c7a4d524ccaca86ad219d3acee07 which removes progress event counting. (I think I got them all.)