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

Fixed VFS tests #185

Merged
merged 2 commits into from
Jul 31, 2022
Merged

Fixed VFS tests #185

merged 2 commits into from
Jul 31, 2022

Conversation

ajmeese7
Copy link
Member

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

@ajmeese7
Copy link
Member Author

Whelp it worked on my machine, that's strange

@andersevenrud
Copy link
Member

Maybe the Jest version has something to do with it ?

@ajmeese7
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member

Ref:

- run: npm install -g jest@^26

@andersevenrud
Copy link
Member

Also: os-js/osjs-dev-meta#24

@ajmeese7
Copy link
Member Author

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
Copy link
Member

Alrighty.

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

@ajmeese7
Copy link
Member Author

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
Copy link
Member

andersevenrud commented Jul 31, 2022

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 🤦‍♂️ Completely went over my head even after you opened this and I had a second look.

@andersevenrud andersevenrud merged commit 6a8fe4d into os-js:master Jul 31, 2022
@ajmeese7 ajmeese7 deleted the chore/vfs-test-fixes branch July 31, 2022 01:38
@andersevenrud
Copy link
Member

andersevenrud commented Jul 31, 2022

Thanks!

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

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

Successfully merging this pull request may close these issues.

None yet

2 participants