Skip to content

SignalR C++ tests passing and part of build#7160

Merged
BrennanConroy merged 27 commits into
masterfrom
brecon/cppTest
Feb 7, 2019
Merged

SignalR C++ tests passing and part of build#7160
BrennanConroy merged 27 commits into
masterfrom
brecon/cppTest

Conversation

@BrennanConroy
Copy link
Copy Markdown
Member

@BrennanConroy BrennanConroy commented Jan 31, 2019

Went through all failing tests and fixed them or removed them if they were no longer relevant.

Also added the client to the build. @halter73 and @mikaelm12 it might be a good idea to pull down the branch and run build.cmd from the signalr root to see if you have the required VS settings.

BrennanConroy
BrennanConroy commented Jan 31, 2019
View reviewed changes
Comment thread src/Servers/IIS/AspNetCoreModuleV2/CommonLibTests/CommonLibTests.vcxproj
natemcmaster
natemcmaster reviewed Jan 31, 2019
View reviewed changes
Comment thread src/Servers/IIS/AspNetCoreModuleV2/CommonLibTests/NativeTests.targets
natemcmaster
natemcmaster reviewed Jan 31, 2019
View reviewed changes
Comment thread eng/targets/Cpp.Common.targets Outdated
@Tratcher Tratcher removed their request for review January 31, 2019 18:06
@BrennanConroy BrennanConroy added the area-signalr Includes: SignalR clients and servers label Jan 31, 2019
@BrennanConroy
Copy link
Copy Markdown
Member Author

BrennanConroy commented Feb 4, 2019

@natemcmaster It looks like the <ImportGroup> has run before the nuget packages are restored. Do you have any recommendations on how to fix that?
Line ref: https://github.com/aspnet/AspNetCore/pull/7160/files#diff-36a000fe432a66617010b714fa6a4d13L106

jkotalik
jkotalik reviewed Feb 4, 2019
View reviewed changes
Comment thread src/SignalR/clients/cpp/src/signalrclient/Build/VS/signalrclient.vcxproj Outdated
@natemcmaster
Copy link
Copy Markdown
Contributor

natemcmaster commented Feb 4, 2019 •
edited
Loading

It looks like the <ImportGroup> has run before the nuget packages are restored. Do you have any recommendations on how to fix that?

The problem is your projects are using a really old version of NuGet and packages.config. It's been a while since I used them, but think these targets work under the expectation that you will have executed NuGet.exe on the .sln before you build. You might try looking into using newer version of NuGet and PackageReference instead.

jkotalik
jkotalik reviewed Feb 4, 2019
View reviewed changes
Comment thread src/SignalR/clients/cpp/src/signalrclient/Build/VS/signalrclient.vcxproj Outdated
@BrennanConroy BrennanConroy changed the base branch from brecon/cppCleanup to master February 4, 2019 21:49
@BrennanConroy
Copy link
Copy Markdown
Member Author

BrennanConroy commented Feb 4, 2019

Thanks for the suggestion Nate! It's building successfully now.

halter73
halter73 reviewed Feb 5, 2019
View reviewed changes
Comment thread src/SignalR/clients/cpp/test/signalrclienttests/internal_hub_proxy_tests.cpp
@BrennanConroy
Copy link
Copy Markdown
Member Author

BrennanConroy commented Feb 7, 2019

Ping!

@BrennanConroy BrennanConroy merged commit 744a32a into master Feb 7, 2019
@BrennanConroy BrennanConroy deleted the brecon/cppTest branch February 7, 2019 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@halter73 halter73 halter73 approved these changes

@davidfowl davidfowl Awaiting requested review from davidfowl

@mikaelm12 mikaelm12 Awaiting requested review from mikaelm12

+3 more reviewers

@pakrym pakrym pakrym left review comments

@natemcmaster natemcmaster natemcmaster approved these changes

@jkotalik jkotalik jkotalik left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-signalr Includes: SignalR clients and servers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@BrennanConroy @natemcmaster @halter73 @pakrym @jkotalik