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

Load wasm files from classpath in generated tests #744

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

andreaTP
Copy link
Collaborator

@yigit follow up from your comment regarding loading files, if I understand the problem correctly, this should generate a test artifact that is self-contained and easier to run using gradle.

Can you confirm that this is going in the right direction?

@andreaTP andreaTP requested a review from evacchi January 20, 2025 12:50
@evacchi
Copy link
Collaborator

evacchi commented Jan 20, 2025

I haven't tested this specifically, but I think the approach should work

@yigit
Copy link
Contributor

yigit commented Jan 20, 2025

yes, this direction should work, let me give it a try in a branch to see if this PR works for runtime-tests project.

@yigit
Copy link
Contributor

yigit commented Jan 20, 2025

yep, this seems to be working 🥳
c7e6bfd
https://github.com/yigit/chicory/actions/runs/12874204085 (it broke artifact upload, need to check)

I've run that locally, 120 tests, 15 failures. Caused by failures similar to this:

Caused by: java.lang.NoSuchMethodError: No static method getLogger(Ljava/lang/String;)Ljava/lang/System$Logger; in class Ljava/lang/System; or its super classes (declaration of 'java.lang.System' appears in /apex/com.android.art/javalib/core-oj.jar)
at com.dylibso.chicory.log.SystemLogger.<clinit>(SystemLogger.java:4)

In that change, i'm adding resources in Gradle. I think this is more reason to delegate to maven to generate the test.jar as discussed in #743, so it is can be source of truth for test jars.

@andreaTP
Copy link
Collaborator Author

Uhm, SystemLogger should not be used unless using wasi, I'd love to be able to reproduce locally to better bisect the issue.

That said, if building a test.jar in Maven works, it is definitely a good approach to the problem (to be good scouts we should leave a couple of lines detailing how it works but can be done later too).

We can merge this and, please, let me know how I can assist on the Maven side!

@andreaTP andreaTP merged commit dc0bb76 into dylibso:main Jan 21, 2025
16 checks passed
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.

3 participants