Update test and build infrastructure #430

cloudRoutine · 2017-03-22T00:40:47Z

Some cleanup and build process enhancement and an upgrade to NUnit3

matthid · 2017-03-22T06:11:55Z

can you rebase and look into the.version3 branch please? I think we already upgraded to nunit3 somewhere...

matthid · 2017-03-22T06:32:26Z

Maybe related? fsharp/fsharp#684

cloudRoutine · 2017-03-22T07:16:20Z

@matthid I checked out version3 after you merged the razor split PR to start this, what am I supposed to rebase onto?

matthid · 2017-03-22T07:19:52Z

@cloudRoutine sorry my bad, everything is fine.. the mobile view ...

matthid · 2017-03-22T07:23:02Z

Can you merge/compare with #416 there we have the working version of the nunit upgrade :)

cloudRoutine · 2017-03-22T07:27:19Z

I've already covered more than that (targets and fsproj config changes so builds don't break on machines with vs2017 installed) and my PR takes into account all the changes to accommodate the Razor split. I didn't move all of the FsUnit tests to the latest syntax, just the stuff that had to changed.

cloudRoutine · 2017-03-22T07:51:31Z

fixing the CI configuration is all that's left, Travis thwarts me once again 💢

matthid · 2017-03-22T08:55:23Z

@cloudRoutine I don't think I'm ready for the VS2017 upgrade ;). Can we make it compatible with both?

matthid · 2017-03-22T08:56:28Z

With both I mean the latest VS2015 and VS2017...

cloudRoutine · 2017-03-22T09:43:53Z

the vs 2017 install puts msbuild at a different path so the targets don't resolve properly, fixing the target resolution so it works in light of this change doesn't affect the vs 2015 compatibility.

cloudRoutine · 2017-03-22T12:39:28Z

@matthid now it's good on windows with vs2017 and the CI, the annoying part is done so now the real work can begin 😺

matthid · 2017-03-22T13:44:25Z

@cloudRoutine perfect, thanks. Can you please merge the related conflicting PR so I don't have to resolve conflicts later?

cloudRoutine · 2017-03-22T13:46:05Z

just don't merge that one, what's the point of doing so?

matthid · 2017-03-22T13:46:57Z

?

matthid · 2017-03-22T13:47:41Z

I think some changes are conflicting and some are not. I don't want to miss some...

cloudRoutine · 2017-03-22T13:48:16Z

there's no real benefit to the other one anymore, it's outdated compared to this, you'd just be creating a headache for yourself trying to merge the other one and there wouldn't be an advantage to doing so

matthid · 2017-03-22T18:38:48Z

@cloudRoutine sorry but did you review and compare? For example FSUnit was upgraded and extracted via paket files...

matthid · 2017-03-22T18:40:15Z

Ok you did the same. I'm confused :)

cloudRoutine · 2017-03-22T18:41:48Z

yea I reviewed it, that's how I knew there was no point in using the other one anymore 😉

matthid · 2017-03-22T18:43:22Z

I'm wondering why did you not start from there?

cloudRoutine · 2017-03-22T18:46:02Z

because it was a PR onto master a year ago and you just merged some big changes into version3 and it was old enough that most of what had been done would just need to be redone

cloudRoutine · 2017-03-22T18:46:20Z

also I had no idea it even existed XD

matthid · 2017-03-22T18:47:49Z

OK :). Anyway thanks for this. Should this be merged or do you want do do some other things?

cloudRoutine · 2017-03-22T18:49:04Z

merged please, I want to do the other stuff in smaller stages. this is big enough as it is

matthid

Some questions asked. But overall looks good.

matthid · 2017-03-22T18:48:57Z

tests/FSharp.MetadataFormat.Tests/files/crefLib/crefLib2.fsproj

+      <PropertyGroup>
+        <FSharpTargetsPath>$(MSBuildExtensionsPath32)\..\Microsoft SDKs\F#\4.0\Framework\v4.0\Microsoft.FSharp.Targets</FSharpTargetsPath>
+      </PropertyGroup>
+    </When>


Should this be the first when clause ?

I changed the order based on other fsproj I found that were successfully building on Travis, the projects are all targeting FSharp.Core 4.0 so it makes sense to me.

matthid · 2017-03-22T18:51:39Z

tests/FSharp.Markdown.Tests/CommonMarkSpecTest.fs

+    > 
+    > Here's some example code:
+    > 
+    >     return shell_exec("echo $input | $markdown_script");


Why change this test?

matthid · 2017-03-22T18:54:28Z

paket.dependencies

 redirects: on

 nuget FSharp.Data
 nuget FAKE
 nuget CommandLineParser
-nuget FSharp.Core
+nuget FSharp.Core 4.0.0.1 redirects:force


I assume upgrading this did lead to errors (same as FSharp.Compiler.Service)?

there were some binding redirect issues that popped up like NUnit was the problem initially

NUGET remote: https://api.nuget.org/v3/index.json CommandLineParser (1.9.71) FAKE (4.56) FSharp.Compiler.Service (2.0.0.6) FSharp.Core (4.0.0.1) - redirects: force FSharp.Data (2.3.2)

I didn't upgrade FCS though

matthid · 2017-03-22T18:55:11Z

build.fsx

+//      Excludes = [] }
+//    |> MSBuildRelease "" "Build"
+//    |> ignore
+//)


Do we enable this later?

nah, forgot to delete it

matthid · 2017-03-22T18:55:28Z

build.fsx

@@ -1,3 +1,5 @@
+System.IO.Directory.SetCurrentDirectory __SOURCE_DIRECTORY__
+


Just wondering... Does this need to be on top now?

ideally setting the directory should be the first thing that you do, i changed the way it's done because the old environment way doesn't work on netcore

I think after opening the namespaces works as well and makes the code shorter :).

you have to load the packages before you can open the namespaces, you need to set the dir if you want to load the packages properly, this is to account for the weird working dir that fsi is set to automatically when you use it in VS

To be honest I don't even remember why its needed (I'm pretty sure the author of this). Now you are :)

matthid · 2017-03-22T19:19:51Z

tests/FSharp.Markdown.Tests/CommonMarkSpecTest.fs

+> 
+> Here's some example code:
+> 
+>     return shell_exec("echo $input | $markdown_script");


Maybe your editor prevents you from using tab characters? But they are required in this repository for some tests...

yup that's what it was

Just be careful with this setting in this repository. Whitespace (at least within strings) matters a lot here :)

cloudRoutine added 3 commits March 21, 2017 15:36

update sln contents

4154739

update test infrastructure

975b6b9

fix test reversal

26c7782

cloudRoutine force-pushed the version3-tests branch from 2d25327 to 26c7782 Compare March 22, 2017 07:50

adjust target resolution

b529526

cloudRoutine force-pushed the version3-tests branch from 6e94e84 to b529526 Compare March 22, 2017 08:09

cloudRoutine added 3 commits March 22, 2017 06:46

adjust target resolution again

3dfb9ec

paket magic mode

f4b82c0

use nuget FSharp.Core all projects

6bb556c

cloudRoutine force-pushed the version3-tests branch from dce443a to a3c7783 Compare March 22, 2017 12:29

fix windows build for vs2017/msbuild15 install

381216e

cloudRoutine force-pushed the version3-tests branch from a3c7783 to 381216e Compare March 22, 2017 12:30

cloudRoutine changed the title ~~[WIP] Update test infrastructure~~ Update test and build infrastructure Mar 22, 2017

matthid mentioned this pull request Mar 22, 2017

Update build script to run tests, updating packages. #416

Closed

matthid approved these changes Mar 22, 2017

View reviewed changes

matthid reviewed Mar 22, 2017

View reviewed changes

cleanup

21eb2f4

cloudRoutine force-pushed the version3-tests branch from 05e2a93 to 21eb2f4 Compare March 22, 2017 19:22

matthid merged commit 5add72a into fsprojects:version3 Mar 22, 2017

		@@ -1,3 +1,5 @@
		System.IO.Directory.SetCurrentDirectory __SOURCE_DIRECTORY__

Update test and build infrastructure #430

Update test and build infrastructure #430

Conversation

cloudRoutine commented Mar 22, 2017

matthid commented Mar 22, 2017

matthid commented Mar 22, 2017

cloudRoutine commented Mar 22, 2017

matthid commented Mar 22, 2017

matthid commented Mar 22, 2017

cloudRoutine commented Mar 22, 2017

cloudRoutine commented Mar 22, 2017

matthid commented Mar 22, 2017

matthid commented Mar 22, 2017

cloudRoutine commented Mar 22, 2017 • edited Loading

cloudRoutine commented Mar 22, 2017

matthid commented Mar 22, 2017

cloudRoutine commented Mar 22, 2017

matthid commented Mar 22, 2017

matthid commented Mar 22, 2017

cloudRoutine commented Mar 22, 2017

matthid commented Mar 22, 2017

matthid commented Mar 22, 2017

cloudRoutine commented Mar 22, 2017

matthid commented Mar 22, 2017

cloudRoutine commented Mar 22, 2017

cloudRoutine commented Mar 22, 2017

matthid commented Mar 22, 2017

cloudRoutine commented Mar 22, 2017

matthid left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloudRoutine commented Mar 22, 2017 •

edited

Loading