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

[mono] Add functional tests for the supported platforms #46286

Merged
merged 28 commits into from
Jan 6, 2021

Conversation

MaximLipnin
Copy link
Contributor

@MaximLipnin MaximLipnin commented Dec 21, 2020

Part of #43865

The functional tests:

  • live under src/tests/FunctionalTests directory.
  • are built as a part of the library tests build.
  • are isolated from the build/test setup used for the tests in src/tests
  • run on CI
  • are extracted into individual test cases:
  • iOS/tvOS device + simulator
    • Interpreter only
    • AOT only
  • Android device + emulator
  • WebAssembly - in progress
    • Interpreter only
    • AOT only

@ghost
Copy link

ghost commented Dec 21, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Part of #43865

  • iOS/tvOS device + simulator - in progress
    • Interpreter only
    • AOT only
  • Android device + simulator - TODO
    • Interpreter only
    • AOT only
  • WebAssembly - TODO
    • Interpreter only
    • AOT only
Author: MaximLipnin
Assignees: -
Labels:

area-System.Net

Milestone: -

@MaximLipnin
Copy link
Contributor Author

Putting the functional tests under src/tests leads to some issues like unrelated test setup (restoring the packages etc), for ex:

/Users/runner/.dotnet/sdk/5.0.101/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(241,5): error NETSDK1004: (NETCORE_ENGINEERING_TELEMETRY=Build) Assets file '/Users/runner/work/1/s/artifacts/tests/coreclr/packages/Common/test_dependencies/test_dependencies/project.assets.json' not found. Run a NuGet package restore to generate this file.

It has to be adjusted in some way.

@MaximLipnin MaximLipnin requested a review from vargaz as a code owner December 25, 2020 15:50
@MaximLipnin MaximLipnin force-pushed the mono_func_tests branch 4 times, most recently from cf7d734 to 8969215 Compare December 30, 2020 07:14
@MaximLipnin MaximLipnin changed the title [WIP][mono] Add functional tests for the supported platforms [mono] Add functional tests for the supported platforms Dec 30, 2020
@mdh1418
Copy link
Member

mdh1418 commented Dec 30, 2020

Many *.cs files are copies but in different folders. Is it possible/reasonable to just compile the same file as part of

<ItemGroup>
  <Compile Include="<filepath>" />
</ItemGroup>

To not need all the duplicate copies of *.cs files? Do we anticipate any modifications that would differentiate the different *.cs files between Interpreter and AOT?

@MaximLipnin
Copy link
Contributor Author

Many *.cs files are copies but in different folders. Is it possible/reasonable to just compile the same file as part of

<ItemGroup>
  <Compile Include="<filepath>" />
</ItemGroup>

To not need all the duplicate copies of *.cs files? Do we anticipate any modifications that would differentiate the different *.cs files between Interpreter and AOT?

Thanks, it sounds reasonable to avoid duplication in the source code. But I'm not sure what those test are going to be like after all, so for the sake of simplicity I'd prefer to have its own source for every particular test for now.

<Import Project="..\..\common.props" />

<!-- Redirect 'dotnet publish' to in-tree runtime pack -->
<Target Name="TrickRuntimePackLocation" AfterTargets="ProcessFrameworkReferences">
Copy link
Member

Choose a reason for hiding this comment

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

This should be in a common targets like the other platforms.

<Project>

<!-- Redirect 'dotnet publish' to in-tree runtime pack -->
<Target Name="TrickRuntimePackLocation" AfterTargets="ProcessFrameworkReferences">
Copy link
Member

Choose a reason for hiding this comment

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

We could have this in a common to all platforms targets file and then just set a property in each project with whatever the PackageDirectory value is. Feels nit picky though.

Copy link
Member

@steveisok steveisok left a comment

Choose a reason for hiding this comment

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

Outside of a couple small nits, looks good. I would add Android AOT in a follow up PR.

Copy link
Member

@mdh1418 mdh1418 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@steveisok
Copy link
Member

I think my comments can be addressed in a follow up PR.

@steveisok steveisok merged commit e3617eb into dotnet:master Jan 6, 2021
@danmoseley
Copy link
Member

cc @eerhardt fyi

@MaximLipnin MaximLipnin deleted the mono_func_tests branch January 8, 2021 06:56
@karelz karelz added this to the 6.0.0 milestone Jan 26, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 25, 2021
@ViktorHofer
Copy link
Member

ViktorHofer commented Jan 3, 2023

Just noticed that we have these functional tests under src/tests but run them as part of the libraries test build:

live under src/tests/FunctionalTests directory.
are built as a part of the library tests build.
are isolated from the build/test setup used for the tests in src/tests

What was the reason for not just moving them into src/libraries?

@akoeplinger
Copy link
Member

@ViktorHofer as far as I can remember the plan was to untangle the dependency on the libraries test infra but it apparently never happened :)

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

Successfully merging this pull request may close these issues.

9 participants