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

DumpIO support #344

Merged
merged 18 commits into from Jun 28, 2018

Conversation

Projects
None yet
2 participants
@kblok
Copy link
Owner

kblok commented Jun 25, 2018

closes #285

kblok added some commits Jun 24, 2018

@kblok kblok changed the title Features/dumpio [WIP] DumpIO support Jun 25, 2018

kblok added some commits Jun 26, 2018

@kblok kblok changed the title [WIP] DumpIO support DumpIO support Jun 27, 2018

@kblok kblok requested a review from Meir017 Jun 27, 2018

Assert.True(success);
}

private string GetDumpIOAppDirectory()

This comment has been minimized.

@Meir017

Meir017 Jun 27, 2018

Collaborator

could we use the TestUtils.FindParentDirectory method here?

process.StartInfo.UseShellExecute = false;
process.StartInfo.WorkingDirectory = GetDumpIOAppDirectory();
process.StartInfo.FileName = "dotnet";
process.StartInfo.Arguments = $" PuppeteerSharp.Tests.DumpIO.dll {dumpioTextToLog} " +

This comment has been minimized.

@Meir017

Meir017 Jun 27, 2018

Collaborator

remove space in the beginning of string

@Meir017

This comment has been minimized.

shouldn't this be based on current target framework? since we are testing netcoreapp and netframework (this would mean we would need to compile the PuppeteerSharp.Tests.DumpIO project for both targets)

This comment has been minimized.

Copy link
Owner

kblok replied Jun 27, 2018

That's true :/

kblok added some commits Jun 27, 2018

@kblok

This comment has been minimized.

Copy link
Owner

kblok commented Jun 28, 2018

Done @Meir017. It was a little tricky, but thanks for pushing!

return process;
}

private string GetDumpIOAppDirectory(DirectoryInfo baseDir)

This comment has been minimized.

@Meir017

Meir017 Jun 28, 2018

Collaborator

maybe instead of relying on the baseDir we could extract the version from the dll itself or something like this https://github.com/cake-build/cake/blob/develop/src/Cake.Core/Polyfill/EnvironmentHelper.cs#L119

kblok added some commits Jun 28, 2018

Ups
@kblok

This comment has been minimized.

Copy link
Owner

kblok commented Jun 28, 2018

Done @Meir017. I never was a fan of conditional compiling, till now :p
You also made me realize this #348

@kblok kblok merged commit 6674705 into master Jun 28, 2018

2 checks passed

CodeFactor No issues found.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@kblok kblok deleted the features/dumpio branch Jun 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment