search

os-js / osjs-client

OS.js Client Module
https://manual.os-js.org/
Other
30 stars 30 forks source link

Fixed VFS tests #185

Closed ajmeese7 closed 1 year ago

ajmeese7 commented 1 year ago

This modification to the test code seems to have resolved the comparison between the pieces of data, resolving the FIXME segments.

ajmeese7 commented 1 year ago

Whelp it worked on my machine, that's strange

andersevenrud commented 1 year ago

Maybe the Jest version has something to do with it ?

ajmeese7 commented 1 year ago

A definite possibility, I have the latest version locally so that is probably it. Would you be willing to update the CI jest version, or are you using the current version for a reason?

andersevenrud commented 1 year ago

It's version locked in the CI because latest Jest seems to trigger some kind of edge case that leaves certain tests hanging. Haven't had time to look into it. Might actually just be a thing in the server codebase come to think of it.

Ideally it should be updated, and then be put into https://github.com/os-js/osjs-dev-meta instead of being in the CI stages.

andersevenrud commented 1 year ago

Ref: https://github.com/os-js/osjs-client/blob/888b8b909ca9e48bc8297469e28b311f2a3ef0a9/.github/workflows/test.yml#L12

andersevenrud commented 1 year ago

Also: https://github.com/os-js/osjs-dev-meta/issues/24

ajmeese7 commented 1 year ago

I was mistaken, my changes did not fix the issue, my jest apparently gave a fluke positive one run then immediately went back to failing. If I can find a different resolution I will come back and update this PR.

andersevenrud commented 1 year ago

Alrighty.

Feel free to look at the dev-meta issue. Would be nice to get that one sorted :blush:

ajmeese7 commented 1 year ago

I figured it out, the conversion from the buffer to string has the potential to include additional null bytes at the end, I believe only when the string is an odd length but I am not 100% sure. I have implemented a regex fix that should resolve this for the purposes of the tests, which I will push shortly.

andersevenrud commented 1 year ago

the conversion from the buffer to string has the potential to include additional null bytes at the end

Man... now that you say this, it's so obvious :man_facepalming: Completely went over my head even after you opened this and I had a second look.

andersevenrud commented 1 year ago

Thanks!

I squashed this and amended the commit message with additional information.