search

reasonml-community / bs-express

Express bindings in Reason
MIT License
210 stars 60 forks source link

Tests fail on master locally #74

Closed maxkorp closed 4 years ago

maxkorp commented 4 years ago

Trying to extend the json middleware to take fileType, which itself seems straight forward, but I wanted to add tests as well, and on master even before my changes the tests fail.

Most of the tests get back a # Netscape HTTP Cookie File where there is not one on the reference data, and this is the only change I can find between the reference.data contents and the output in test.data. I've included test.data as a txt below.

test.data.txt

ncthbrt commented 4 years ago

Hi Max, I'll only be able to look at this on Sunday. Hope that's not too much of a blocker?

On Thu, 23 Jan 2020, 00:30 Max Korp, notifications@github.com wrote:

  • MacOS 10.15.2 (19C57)
  • Node 12.13.1
  • Curl 7.64.1

curl 7.64.1 (x86_64-apple-darwin19.0) libcurl/7.64.1 (SecureTransport) LibreSSL/2.8.3 zlib/1.2.11 nghttp2/1.39.2 Release-Date: 2019-03-27 Protocols: dict file ftp ftps gopher http https imap imaps ldap ldaps pop3 pop3s rtsp smb smbs smtp smtps telnet tftp Features: AsynchDNS GSS-API HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz MultiSSL NTLM NTLM_WB SPNEGO SSL UnixSockets

Trying to extend the json middleware to take fileType, which itself seems straight forward, but I wanted to add tests as well, and on master even before my changes the tests fail.

Most of the tests get back a # Netscape HTTP Cookie File where there is not one on the reference data, and this is the only change I can find between the reference.data contents and the output in test.data. I've included test.data as a txt below.

test.data.txt https://github.com/reasonml-community/bs-express/files/4100158/test.data.txt

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/reasonml-community/bs-express/issues/74?email_source=notifications&email_token=ACCJEWLESF4HVBXNUHRKOKDQ7DCG3A5CNFSM4KKNSD2KYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IIC2A6A, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCJEWMUT2IM36HIAOFBQ43Q7DCG3ANCNFSM4KKNSD2A .

ncthbrt commented 4 years ago

@maxkorp I've pulled locally, and it appears that it's an environment specific issue you're facing. I have not been able to reproduce your issue.

I'm going to close this for now, but happy to reopen the issue if more information arises.

maxkorp commented 4 years ago

@ncthbrt may I ask what os etc you're running on? Just trying to track this down so I can write tests to go with my PR. I may try running them inside a docker container.