wix-incubator / kissfs

Extensible and reactive text-based file-system library that keeps it simple, universal and cross-platform
MIT License
12 stars 5 forks source link

Remove Bluebird in favor of native Promises #85

Closed AviVahl closed 7 years ago

AviVahl commented 7 years ago

This PR should fix the "Core3 Regular Test".

tyv commented 7 years ago

Awesome! p.s. most likely extending timeout wouldn't help, I tried this in numerous variations.

AviVahl commented 7 years ago

@idoros reviewed this. (just realized this was not reflected in the PR)

and yes, timeout didn't help... disabled macOS in Travis and replaced with our own dedicated mac machine to run test:node on.

amir-arad commented 7 years ago

no need to test src/promise-utils.ts ? @idoros ?

amir-arad commented 7 years ago

I've "optimized" the promise retry loop to "look better" and it passes all tests:

        while (!aborted && --retries >= -10) {
            try {
                const result = await promiseProvider();
                return resolve(result);
            } catch (e) {
                lastError = e;
                if (retries > 0) {
                    await delayedPromise(interval);
                }
            }
        }
amir-arad commented 7 years ago

all in all great job, I'm going to copy some of it to other projects!

idoros commented 7 years ago

no need to test src/promise-utils.ts ?

Missed this and agree, although I would be happy not to write any of these promise helper functions. I'm sure we can find some tiny project to take it from.

amir-arad commented 7 years ago

@idoros There is (or supposed to be) a project that polyfills the Bluebird API on top of es6 promises. @benjamingr can you remind me how it's called?

amir-arad commented 7 years ago

HA! https://github.com/benjamingr/bluebird-api of course. @idoros @AviVahl check it out

amir-arad commented 7 years ago

@AviVahl the test in our CI is flakey too.

82 is a PR with zero changes, it's tests are flaky.

AviVahl commented 7 years ago

Will check