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

[CI] Enable sys tests for NodeJS #10437

Merged
merged 4 commits into from
Oct 19, 2021

Conversation

tobil4sk
Copy link
Member

@tobil4sk tobil4sk commented Oct 17, 2021

Addresses #10436 partially.

Previously, the sys api tests for nodejs were not running. This PR enables most of them, apart from those that cannot be enabled currently because sys.io.Process and sys.net.Socket.bind() are not implemented in hxnodejs.

These hxnodejs PR need to be merged so that the tests run successfully:

@Simn
Copy link
Member

Simn commented Oct 18, 2021

Looks like this needs a few more fixes.

@tobil4sk
Copy link
Member Author

@Simn I submitted another PR to fix the issue with FileSystem.absolutePath() so that test is now fully enabled. If the hxnodejs PRs mentioned above are merged and the changes are released, then these tests will stop failing. The only ignored tests on hxnodejs are now for sys.io.Process and sys.net.Socket.bind(), as these APIs are missing/incomplete in hxnodejs.

@tobil4sk
Copy link
Member Author

Thanks, just to clarify these are the other two PRs that will need to be merged as well apart from that one:

@tobil4sk
Copy link
Member Author

File.append() isn't working in hxnodejs on Mac...

@Simn
Copy link
Member

Simn commented Oct 18, 2021

That particular file already has a lot of conditional compilation for stuff not working on various targets. Feel free to do that for this case as well for the time being. That way we can merge this PR now and track the issue in an issue.

@tobil4sk
Copy link
Member Author

Disabled it on Mac, and mentioned the issue in the checklist #10436. Might be good for those to be moved into separate issues in hxnodejs maybe.

@Simn Simn merged commit 359beb0 into HaxeFoundation:development Oct 19, 2021
@Simn
Copy link
Member

Simn commented Oct 19, 2021

CI finally behaved. Thank you for this contribution!

@skial skial mentioned this pull request Oct 20, 2021
1 task
@tobil4sk
Copy link
Member Author

Great, thanks for your time!

@tobil4sk tobil4sk deleted the nodejs-sys-test branch October 20, 2021 18:56
@tobil4sk tobil4sk mentioned this pull request Oct 20, 2021
6 tasks
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.

2 participants