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

Added tests for netcoreapp1.1 #270

Merged
merged 6 commits into from
Dec 13, 2016
Merged

Added tests for netcoreapp1.1 #270

merged 6 commits into from
Dec 13, 2016

Conversation

harshjain2
Copy link
Contributor

@harshjain2 harshjain2 commented Dec 10, 2016

Added tests for netcoreapp1.1.
#167

@harshjain2
Copy link
Contributor Author

@dotnet-bot test this please

@codito
Copy link
Contributor

codito commented Dec 11, 2016

Any reason to choose new test asset projects? We could multitarget existing test asset to <TargetFrameworks>net46,netcoreapp1.0,netcoreapp1.1</TargetFrameworks>.

@harshjain2
Copy link
Contributor Author

@codito My initial thought was to keep separate folder, but it seems like we will end up having so many folders as netcoreapp evolve further.
I will multi-target existing test asset projects.

@@ -2,8 +2,9 @@
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" />

<PropertyGroup>
<TargetFramework>net46</TargetFramework>
<TargetFrameworks>net46;netcoreapp1.1</TargetFrameworks>
Copy link
Contributor

Choose a reason for hiding this comment

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

We want to support netcoreapp1.0 as well.

@@ -2,9 +2,9 @@
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" />

<PropertyGroup>
<TargetFrameworks>netcoreapp1.0;net46</TargetFrameworks>
<TargetFrameworks>netcoreapp1.0;net46;netcoreapp1.1</TargetFrameworks>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to modify the e2e test for running this test project targeting netcoreapp1.1?

/// <summary>
/// The custom data test method attribute.
/// </summary>
public class CustomDataTestMethodAttribute : TestMethodAttribute
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesomeness!

{
AcceptanceTestBase.SetTestEnvironment(this.testEnvironment, runnerFramework, targetFramework, targetRuntime);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this move to TestInitialize?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention behind keeping it here is that the parameters for method will come from data rows.

<PackageTargetFallback Condition=" '$(TargetFramework)' == 'netcoreapp1.0' ">$(PackageTargetFallback);dnxcore50;portable-net45+win8</PackageTargetFallback>
<OutputType Condition=" '$(TargetFramework)' == 'netcoreapp1.0' ">Exe</OutputType>
<PackageTargetFallback Condition=" '$(TargetFramework)' == 'netcoreapp1.0' Or '$(TargetFramework)' == 'netcoreapp1.1' ">$(PackageTargetFallback);dnxcore50;portable-net45+win8</PackageTargetFallback>
<OutputType Condition=" '$(TargetFramework)' == 'netcoreapp1.0' Or '$(TargetFramework)' == 'netcoreapp1.1' ">Exe</OutputType>
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to invert this to compare with net46.

@codito
Copy link
Contributor

codito commented Dec 13, 2016

👍

@harshjain2 harshjain2 merged commit 9a04658 into microsoft:master Dec 13, 2016
@codito codito added this to the Preview 111.1 - 20161217 milestone Dec 19, 2016
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