Wait for writing to tmp fiels before handling zip stream close #1093
Conversation
There was a problem hiding this comment.
So if I understand this correctly, this is a race condition in which we could iterate through each entry and trigger the close event before the tmp callback is triggered, so therefore this.waitingWorkSheets would be empty? I can't think of a way of testing this, but good catch
|
I moved resolving promise to finish listener of tempStream to handle a rare case when zip close event occurs before |
|
@Siemienik @guyonroche I think we should merge this, but I'm not sure if we can write a test here. What do you think? |
|
This is not a perfect practice to commit something without tests. mhmm... could you test emitting error? (Do you know any situation that should throw an exception?) |
|
Maybe we could use proxyquire? I think a tmp failure is really unlikely, the issue as I understand it is actually that the tmp callback could potentially get called AFTER the zip's close event has already been triggered, so |
|
@Siemienik I added one test. I totally understand that this is not good practice to commit something without tests but as I mentioned I could not figurated out how to do that properly, please let me know if you have some better ideas to test it. |
I'm having a hard time to write a failing test without those changes but in one commercial project, we have tests that were flaky without this.