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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Python bindings and test them in CI #2086

Closed

Conversation

peace-maker
Copy link
Contributor

There is a test setup for the python bindings in the bindings/Makefile, which compare the output of the /tests/test_<arch> programs with the equivalent bindings/python/test_<arch>.py scripts. They weren't run in CI and the tests were heavily out of sync.

I've fixed all issues with the Python bindings that popped up while fixing the tests. This resulted in new bindings for the wasm and sh targets. 馃帀

I'm aware of the plans to unify the tests using cstest, but figured any tests for the bindings at all are better than nothing in the meantime. The Java bindings are heavily outdated, so I've disabled tests for them until they're taken care of.

@XVilka
Copy link
Contributor

XVilka commented Jul 14, 2023

@peace-maker thank you! Despite plans to unify tests, testing bindings separately is also required, so it's not a duplicate work anyway: #2043

@XVilka XVilka mentioned this pull request Jul 11, 2023
24 tasks
@XVilka
Copy link
Contributor

XVilka commented Jul 14, 2023

I think it would be a perfect fit for the bug fix release if it happens: #2081

@XVilka
Copy link
Contributor

XVilka commented Jul 17, 2023

@kabeor @aquynh

@aquynh
Copy link
Collaborator

aquynh commented Jul 18, 2023

looks nice, but do you mind splitting this PR into a few independent PRs, instead of mixing all of them into one?

@XVilka
Copy link
Contributor

XVilka commented Jul 20, 2023

Resulting PRs:

Should be imported also to the v5 branch (except auto-sync-related commit, of course), IMHO.

@peace-maker
Copy link
Contributor Author

I'll wait with the testing part until the other PRs are merged since it depends on them.

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

3 participants