nodeSolidServer / node-solid-server

Solid server on top of the file-system in NodeJS
https://solidproject.org/for-developers/pod-server
Other
1.77k stars 297 forks source link

Fix/issue#1502 #1782

Closed zg009 closed 2 months ago

zg009 commented 2 months ago

Fixes all the errors in #1502.

-ldp-test.js -account-manager-test.js -account-template-test.js -acl-*-test.js

Unfortunately, Node 15 is no longer supported. However, I tested with node v16.14.2 on WSL running npm v 9.5.2. It looks like all tests which failed can now pass successfully. This does not cover the ENTIRE test suite, just those tagged with "1502".

zg009 commented 2 months ago

Actually there are 4 tests in ldp put that need some information: Are they relic tests which passed years ago and no longer should do to specification changes?

bourgeoa commented 2 months ago

Nice work.

I unskipped the 4 remaining skipped test in ldp-test.js and all passed on node 16/18 wsl 1 and wsl 2 wsl 2 has an other issue with

       POST (multipart)
         should create as many files as the ones passed in multipart:
     Error: Either the size (remote/local) don't match or files are not stored

Unskipping the issues should resolve all 1502 issue

zg009 commented 2 months ago

I should have been more clear Alain.

Those 4 tests "pass", however if you change the assertions to any error status, they still pass. Try assert.equal(err.status, 1234), it will still say success.

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: Alain Bourgeois @.> Sent: Sunday, April 21, 2024 5:29:00 AM To: nodeSolidServer/node-solid-server @.> Cc: Zachary Grider @.>; Author @.> Subject: Re: [nodeSolidServer/node-solid-server] Fix/issue#1502 (PR #1782)

Nice work.

I unskipped the 4 remaining skipped test in ldp-test.js and all passed on node 16/18 wsl 1 and wsl 2 wsl 2 has an other issue with

   POST (multipart)
     should create as many files as the ones passed in multipart:
 Error: Either the size (remote/local) don't match or files are not stored

Unskipping the issues should resolve all 1502 issue

— Reply to this email directly, view it on GitHubhttps://github.com/nodeSolidServer/node-solid-server/pull/1782#issuecomment-2067994527, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AYZIGJNNV7QCQKKSSHKCYWDY6OIGZAVCNFSM6AAAAABGRCD5A6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRXHE4TINJSG4. You are receiving this because you authored the thread.Message ID: @.***>

bourgeoa commented 2 months ago

@zg009 You are right these tests do not test anything. It may have been a test concept or they may have been replaced when moving the quota to /utils It is before I begun any work on NSS

We may remove them and replace with some integration tests. I don't know/remember if quota is working in practice.

zg009 commented 2 months ago

Storagequota is working as intended.

Sent from my Verizon, Samsung Galaxy smartphone Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: Alain Bourgeois @.> Sent: Monday, April 22, 2024 12:42:47 PM To: nodeSolidServer/node-solid-server @.> Cc: Zachary Grider @.>; Mention @.> Subject: Re: [nodeSolidServer/node-solid-server] Fix/issue#1502 (PR #1782)

@zg009https://github.com/zg009 You are right these tests do not test anything. It may have been a test concept or they may have been replaced when moving the quota to /utils It is before I begun any work on NSS

We may remove them and replace with some integration tests. I don't know/remember if quota is working in practice.

— Reply to this email directly, view it on GitHubhttps://github.com/nodeSolidServer/node-solid-server/pull/1782#issuecomment-2070391639, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AYZIGJMBXNCWVWSOLNJJUF3Y6VDZLAVCNFSM6AAAAABGRCD5A6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZQGM4TCNRTHE. You are receiving this because you were mentioned.Message ID: @.***>