Update pubnub and clear logs

balena-io-modules / resin-device-logs

Resin.io device logs utilities.

Apache License 2.0

3 stars 2 forks source link

Update pubnub and clear logs #19

Closed emirotin closed 7 years ago

emirotin commented 7 years ago

Closes #17 #18

So code-wise I believe it's ready. I've switched the tests from mocks (that I truly find useless here because they mask the real API) to using the actual PubNub with random channel names. And actually these tests just helped me find the bug in the code.

Now there are two problems with them: 1) they fail in the browser. I think because PubNub history does not guarantee we receive everything published so far (even though I do wait on successful publish before trying to retrieve the history). So I've added some pretty random delays and they work in node but not in phantom. 2) tests fail on Windows (AppVeyor) and I don't know why

emirotin commented 7 years ago

The tests are unstable and new functionality (clear) is not covered yet, but the body of the code is OK to review

emirotin commented 7 years ago

Ah OK seems the windows failure is now the same browser failure (it used to be something different in the node part).

The browser failure is essentially history getting 0 results when it's supposed to get 3. Will try playing with delays as I don't have any better ideas.

AH, and of course I've tested this by hand in the real UI and it works fine.

emirotin commented 7 years ago

Should I just disable the automated browser tests by excluding them from the default test script? They run locally and we also test it indirectly in the UI, but they fail consistently in the CI.

pimterry commented 7 years ago

I don't think we should disable the tests - we'll forget about them pretty quickly if we do. We should get them fixed, but that doesn't have to block things. If we need this out in time to make the SDK 6.0 release, and we've tested this manually, then I think this is good to merge now, and we can come back and improve the tests later (and it'll be easier to remember to do that if they stay failing).