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.

Sign up for GitHub

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

Jump to bottom

Implementing 'AddResultFile' for NetCore TestContext #609

Merged
merged 8 commits into from
Apr 29, 2019
Merged

Implementing 'AddResultFile' for NetCore TestContext #609

merged 8 commits into from
Apr 29, 2019

Conversation

parrainc
Copy link
Contributor

Fix issue #394

Still work in progress, a little block on PlatformServices.Portable implementation.

@parrainc
Copy link
Contributor Author

@jayaranigarg when you have some time, please check this.
Build is failing because since PlatformService.Portable project is referencing Extension.Core project to get the [NetCore]TestContext, an implementation for AddResultFile() should be provided, but since the framework target version on the PlatformService.Portable project is lower than the one containing the GetFullPath in the Path api, can't use it, hence build fail.

Any suggestions?

jayaranigarg
jayaranigarg reviewed Apr 25, 2019
View reviewed changes
src/Adapter/PlatformServices.Shared/netstandard1.0/Services/ns10TestContextImplementation.cs Outdated
@@ -133,6 +140,16 @@ public UTF.TestContext Context
}
}

public override void AddResultFile(string fileName)
{
if (string.IsNullOrEmpty(fileName))
Copy link
Member

Choose a reason for hiding this comment

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

You can skip adding any code here and keep this function as a no-op for PlaformServices.Portable.
Note - PlatformServices.Portable is used just at compile time, and it will get replaced by PlatformServices.Desktop/Netcore at runtime depending on the target framework for your test project.

Since, desktop TestContext already has a implementation for AddResultsFile(), we just need to add implementation in NetCoreTestContext and we should be good to know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fantastic! good to know that. I'm pushing in a few mins

@jayaranigarg
Copy link
Member

Also, please add tests for your code changes.

@parrainc
Copy link
Contributor Author

parrainc commented Apr 26, 2019 •
edited
Loading

Also, please add tests for your code changes.

yup, didn't push them up but have them here :)

@parrainc parrainc changed the title [WiP] Implementing 'AddResultFile' for NetCore TestContext Implementing 'AddResultFile' for NetCore TestContext Apr 26, 2019
jayaranigarg
jayaranigarg reviewed Apr 26, 2019
View reviewed changes
src/Adapter/PlatformServices.NetCore/Services/NetCoreTestContextImplementation.cs Outdated Show resolved Hide resolved
src/Adapter/PlatformServices.Shared/netstandard1.0/Services/ns10TestContextImplementation.cs Outdated Show resolved Hide resolved
test/UnitTests/PlatformServices.NetCore.Unit.Tests/PlatformServices.NetCore.Unit.Tests.csproj Outdated Show resolved Hide resolved
...ests/PlatformServices.Shared.Unit.Tests/netstandard1.0/ns10TestContextImplementationTests.cs Outdated Show resolved Hide resolved
...ests/PlatformServices.Shared.Unit.Tests/netstandard1.0/ns10TestContextImplementationTests.cs Outdated Show resolved Hide resolved
...nitTests/PlatformServices.Shared.Unit.Tests/netcore/NetCoreTestContextImplementationTests.cs Outdated Show resolved Hide resolved
...nitTests/PlatformServices.Shared.Unit.Tests/netcore/NetCoreTestContextImplementationTests.cs Outdated Show resolved Hide resolved
@jayaranigarg
Copy link
Member

@parrainc : PR looks good overall. Few minor comments, kindly do the needful changes and then we should be good to push this in.

@parrainc
Copy link
Contributor Author

changes already pushed. Thanks for the review.

@jayaranigarg
Copy link
Member

@parrainc : Nice 👍 Thank you for the contribution !!

@jayaranigarg jayaranigarg merged commit c3d352c into microsoft:master Apr 29, 2019
@jayaranigarg jayaranigarg mentioned this pull request Apr 29, 2019
Closed
@stephenjg24-git
Copy link

Hello. Any eta on when ‘addresultfile’ will included in the package from
Night? I has a .netcore22 Test app that I have been working on and I would like use this. I am using https://github.com/microsoft/testfx/releases/tag/v2.0.0-beta4 currently

@jayaranigarg
Copy link
Member

@stephenjg24-git : You can use our latest bits from here:
https://dotnet.myget.org/F/mstestv2/api/v3/index.json

In particular, you should update your adapter and framework nuget package to these versions:
https://dotnet.myget.org/feed/mstestv2/package/nuget/MSTest.TestAdapter/2.0.0-build-20190430-01
https://dotnet.myget.org/feed/mstestv2/package/nuget/MSTest.TestFramework/2.0.0-build-20190430-01

@stephenjg24-git
Copy link

@jayaranigarg, when I am in Visual Studio I can user the builds you note above,.

What i can not do is use this in our build pipeline, I can work on adding this to the pipeline.

Thanks

@TroyWalshProf TroyWalshProf mentioned this pull request Aug 21, 2019
Closed
This was referenced Mar 17, 2021
Merged
Closed
Closed
Closed
Closed
Closed
@dependabot dependabot bot mentioned this pull request Mar 17, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jayaranigarg jayaranigarg jayaranigarg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@parrainc @jayaranigarg @stephenjg24-git