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

[browser] Add OutputType=Library tests #110731

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Dec 16, 2024

Fixes #110620.

@ilonatommy ilonatommy added arch-wasm WebAssembly architecture os-browser Browser variant of arch-wasm labels Dec 16, 2024
@ilonatommy ilonatommy added this to the 10.0.0 milestone Dec 16, 2024
@ilonatommy ilonatommy self-assigned this Dec 16, 2024
@ilonatommy ilonatommy changed the title [browser] OutputType=Library tests and improvements [browser] OutputType=Library tests to identify active issues Dec 16, 2024
@ilonatommy ilonatommy changed the title [browser] OutputType=Library tests to identify active issues [browser][wasi] OutputType=Library tests to identify active issues Dec 16, 2024
@ilonatommy ilonatommy changed the title [browser][wasi] OutputType=Library tests to identify active issues [browser] Add OutputType=Library tests Dec 17, 2024
@ilonatommy ilonatommy marked this pull request as ready for review December 17, 2024 20:56
@ilonatommy ilonatommy requested a review from Copilot December 19, 2024 10:34

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (4)

src/mono/wasm/testassets/EntryPoints/LibraryMode.cs:3

  • [nitpick] The class name 'Test' is too generic. Consider renaming it to something more descriptive, such as 'LibraryCallbackHandler'.
public unsafe class Test

src/mono/wasm/testassets/EntryPoints/LibraryMode.cs:6

  • [nitpick] The method name 'MyCallback' is too generic. Consider renaming it to something more descriptive, such as 'HandleCallback'.
public static int MyCallback()

src/mono/wasi/Wasi.Build.Tests/WasiLibraryModeTests.cs:22

  • The method LibraryModeBuildPublishRun is now parameterized with sdk and hasWasmAppBundle, but there are no test cases to cover the different behaviors based on these parameters. Consider adding test cases to ensure these scenarios are tested.
[Theory]

src/mono/wasi/testassets/LibraryMode.cs:3

  • [nitpick] The class name 'Test' is too generic. Consider renaming it to something more descriptive like 'LibraryModeTest'.
public unsafe class Test
@pavelsavara
Copy link
Member

please let's discuss the goals of this before we proceed further

@maraf
Copy link
Member

maraf commented Jan 6, 2025

please let's discuss the goals of this before we proceed further

The use-case on browser is something like this demo https://github.com/maraf/dotnet-wasm-react. It doesn't need main, because there is no single entrypoint from .NET point of view. The application is driven by JavaScript

@pavelsavara

This comment was marked as resolved.

@pavelsavara
Copy link
Member

There are updated requirements in #110620 description

@ilonatommy ilonatommy requested a review from maraf January 8, 2025 11:30
Copy link
Member

@maraf maraf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, add also an index.html and main.js to the library mode testasset. The main.js will start the runtime and instead of calling main, it will call just the JSExport. Ideally, also run the app in the browser

@ilonatommy ilonatommy requested a review from maraf January 9, 2025 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly architecture area-VM-meta-mono os-browser Browser variant of arch-wasm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[browser] Add test for OutputType=Library
4 participants