Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failing test on windows 10 #110

Closed
gfellerph opened this issue Dec 23, 2019 · 8 comments
Closed

Failing test on windows 10 #110

gfellerph opened this issue Dec 23, 2019 · 8 comments
Labels
good first issue Good for newcomers

Comments

@gfellerph
Copy link
Contributor

🐛 Bug Report

The multipart test is failing to call finished when both files are pumped.

test/multipart.test.js .............................. 47/48 12s
should call finished when both files are pumped
not ok should be equal
--- wanted
+++ found
-2
+0
compare: ===
at:
line: 97
column: 9
file: test/multipart.test.js
type: Immediate
method: _onImmediate
stack: |
Immediate._onImmediate (test/multipart.test.js:97:9)
source: |
t.equal(fileCount, 2)

total ............................................. 105/106

To Reproduce

Steps to reproduce the behavior:
Clone the repository, run npm install and then npm run test

Your Environment

  • node version: v12.10.0
  • fastify-multipart version: 1.0.3
  • os: Windows 10 Pro 64bit
@mcollina
Copy link
Member

are you able to send a PR to fix it? Thanks!

@gfellerph
Copy link
Contributor Author

I actually don't know if it's just me. If anybody else can reproduce the bug on windows I'll look into it.

@Eomm
Copy link
Member

Eomm commented Dec 23, 2019

Unfortunately, I had this problem and it is a matter of optimization of the PC.

For example if you write this handler in the should call finished when both files are pumped test:

    function handler (field, file, filename, encoding, mimetype) {
      pump(file, new (require('stream').Writable)({
        write (chunk, encoding, done) {
          done()
        }
      }), function (err) {
        t.error(err)
        fileCount++
      })
    }

All will works

Causes: HDD latency + battery saving mode +❓

For this reason, we have windows in our CI 😅

@gfellerph
Copy link
Contributor Author

@Eomm, I tested your code and the test passes now. Should I submit a PR?

@mcollina
Copy link
Member

mcollina commented Dec 23, 2019 via email

@Eomm
Copy link
Member

Eomm commented Dec 23, 2019

Not sure why we saved the files in a temporary directory

Check it out please

@Eomm Eomm added the good first issue Good for newcomers label Feb 26, 2020
@StarpTech
Copy link
Member

StarpTech commented Aug 25, 2020

This should be finally fixed with the new promise API which works flawlessly on windows #140

@StarpTech
Copy link
Member

Closing due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants