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

Support for netstandard1.6 and .NET Core #101

Merged
merged 47 commits into from
Feb 27, 2017
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
1891b5d
Expect.equal netstandard compatible
adamchester Feb 19, 2017
1e6d346
Travis build
adamchester Feb 19, 2017
4d20429
AppVeyor build
adamchester Feb 19, 2017
c85ae6d
Fix global.json for rc4-004771
adamchester Feb 19, 2017
958fe8a
fsproj for rc4-004771
adamchester Feb 19, 2017
0370de5
Stop running FAKE build steps on CI
adamchester Feb 19, 2017
283aa71
Compile Expecto.netcore via Rake build
adamchester Feb 19, 2017
3122e82
Add a comment about travis dotnet support
adamchester Feb 19, 2017
6584664
Reduce netcore errors with #if NETSTANDARD1_6
adamchester Feb 19, 2017
72d6411
Fixup travis file encoding
adamchester Feb 19, 2017
5d52316
Introduce `RESHAPED_REFLECTION`,
adamchester Feb 20, 2017
6e176aa
More fixes
adamchester Feb 20, 2017
f3930ef
Upgrade Mono.Cecil, fix build errors
adamchester Feb 21, 2017
7014fe7
create_nugets_dotnetcli, verbose build
adamchester Feb 23, 2017
35233e7
Undo UTF8 BOM file encoding
adamchester Feb 24, 2017
9b1b7a3
Undo test start argument changes
adamchester Feb 24, 2017
102d34f
Enable detecting console UTF-8 again
adamchester Feb 24, 2017
d763e20
Remove intellisense hack,
adamchester Feb 24, 2017
ce2bee9
5.0.0-beta / Target >= net45
adamchester Feb 25, 2017
95258b3
Thanks to >=net45, less reflection madness
adamchester Feb 25, 2017
2169ced
Travis matrix: mono 4.6.1|latest
adamchester Feb 25, 2017
8f55a7e
Travis: mono 4.8.0 and weekly
adamchester Feb 25, 2017
69cd5f1
Merge remote-tracking branch 'upstream/master' into netcore-rc4
adamchester Feb 25, 2017
50e7e03
Travis: fixed mono allow_failures
adamchester Feb 25, 2017
faeef2a
Improve diff & fsproj encoding
adamchester Feb 25, 2017
e4bbf2c
Run tests on netcore during build
adamchester Feb 26, 2017
4c5da2c
restore before test
adamchester Feb 26, 2017
8d4aefb
restore before tests
adamchester Feb 26, 2017
4c1c280
proj
adamchester Feb 26, 2017
5165571
locate fscheck with nuget.config
adamchester Feb 26, 2017
34883c6
Fix fsproj encoding
adamchester Feb 26, 2017
cc5159f
Fix reflection tests
adamchester Feb 26, 2017
b3c184f
VSCode build/test integration
adamchester Feb 26, 2017
fe77792
file encoding (again)
adamchester Feb 26, 2017
8bd3c2b
Remove FSharpx
adamchester Feb 27, 2017
48c1f4e
Put back nuget.commandline
adamchester Feb 27, 2017
639de38
nuget command line
adamchester Feb 27, 2017
e084eb2
Add ulimit hack for osx restore
adamchester Feb 27, 2017
ec02d07
increase ulimit
adamchester Feb 27, 2017
e5094f1
Improved ulimit comment
adamchester Feb 27, 2017
aa9b3b8
fix logary hash
cloudRoutine Feb 27, 2017
51c1f7f
Merge pull request #1 from cloudRoutine/netcore
adamchester Feb 27, 2017
238cf96
dependency update
cloudRoutine Feb 27, 2017
3432af9
only build automatically, don't run tests
cloudRoutine Feb 27, 2017
c9612cc
Merge pull request #2 from cloudRoutine/netcore
adamchester Feb 27, 2017
18a35a3
generate AssemblyVersionInfo
cloudRoutine Feb 27, 2017
c233b74
Merge pull request #3 from cloudRoutine/netcore
adamchester Feb 27, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 22 additions & 12 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,20 +1,30 @@
---
language: csharp

mono:
- 4.6.1
- latest # => "stable release"
- weekly # => "latest commits"
matrix:
Copy link
Owner

Choose a reason for hiding this comment

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

Nice, but where did mono testing go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's important, that's why I removed it. Would you like me to bring it back? Is it really so worthwhile? Why?

Copy link
Owner

Choose a reason for hiding this comment

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

We primarily build on mono (while .Net Core has been maturing), so it's important to us to see edge cases on the mono runtime in CI before we hit them during normal development. I've established a routine to notify in the mono chat room every time they break something in latest/weekly, which has seemingly worked and they are now saying that the current mono is the most stable ever. I do the same thing in Logary and Suave.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK understood. I will bring back the mono matrix.

include:
- os: linux # Ubuntu 14.04
dist: trusty
sudo: required
dotnet: 1.0.0-preview2-003121
# Note that travis doesn't support dotnet cli rc4 via 'dotnet' yet,
# so we just use the old preview2 support to get things setup
# first, then manually add dotnet CLI RC4 during install.
env:
- CLI_VERSION=1.0.0-rc4-004771
- os: osx # OSX 10.11
osx_image: xcode7.3
dotnet: 1.0.0-preview2-003121
Copy link
Owner

Choose a reason for hiding this comment

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

Why remove?

Choose a reason for hiding this comment

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

Probably because the new csproj format is not really released. It would probably be easier to have it work if it's using project.json to begin with.

Choose a reason for hiding this comment

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

Or using a separate solution for the .net core things in order to be able to keep developing it without interfering with mono testing. It's a pain to maintain, but I guess you could write a short ruby script to keep .net fsproj file references in sync with with the core .net fsproj

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't think it's appropriate to test against future mono versions. I figured you probably just did it because it was easy and nice to have (but not super important to Expecto).

Copy link
Owner

Choose a reason for hiding this comment

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

The things that this often tests is how their GC works; all of my projects are highly concurrent and all create lots of small garbage during runtime. Plus, being F#, there are slightly different ways of traversing the object graph/accessibility from regular C# projects, which sometimes triggers bugs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK understood. I will bring back the mono matrix.

env:
- CLI_VERSION=1.0.0-rc4-004771

install:
- rvm install 2.2.3
- gem install bundler
- sudo apt-get update -y
- rvm install 2.2.3
- gem install bundler
- export DOTNET_INSTALL_DIR="$PWD/.dotnetcli"
- curl -sSL https://raw.githubusercontent.com/dotnet/cli/rel/1.0.0/scripts/obtain/dotnet-install.sh | bash /dev/stdin --version "$CLI_VERSION" --install-dir "$DOTNET_INSTALL_DIR"
- export PATH="$DOTNET_INSTALL_DIR:$PATH"

script:
- bundle && bundle exec rake

matrix:
allow_failures:
- mono: latest
- mono: weekly
# - ./build.sh
2 changes: 1 addition & 1 deletion Expecto.Tests/Expecto.Tests.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<WarningLevel>3</WarningLevel>
<PlatformTarget>AnyCPU</PlatformTarget>
<DocumentationFile>bin\Debug\Expecto.Tests.xml</DocumentationFile>
<StartArguments>--stress 1</StartArguments>
<StartArguments>--filter performance</StartArguments>
Copy link
Owner

Choose a reason for hiding this comment

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

Why change? I'm not sure we want to filter to performance tests only.

</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
<DebugType>pdbonly</DebugType>
Expand Down
62 changes: 20 additions & 42 deletions Expecto.netcore/Expecto.netcore.fsproj
Original file line number Diff line number Diff line change
@@ -1,54 +1,32 @@
<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" />
<Project Sdk="FSharp.NET.Sdk;Microsoft.NET.Sdk">

<PropertyGroup>
<AssemblyName>Expecto</AssemblyName>
<Version>1.1.2</Version>
<OutputType>Library</OutputType>
<TargetFramework>netstandard1.6</TargetFramework>
<DebugType>pdbonly</DebugType>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
<DefineConstants>$(DefineConstants);RELEASE</DefineConstants>
<DefineConstants Condition=" '$(TargetFramework)' == 'netstandard1.6' ">
Copy link
Contributor

Choose a reason for hiding this comment

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

just a style note (i learned that recently and i like that), using multiline help with changes later (less diff) and to read list if long.

<DefineConstants Condition=" '$(TargetFramework)' == 'netstandard1.6' ">
    NETSTANDARD1_6;
    RESHAPED_REFLECTION;
    @(DefineConstants)
</DefineConstants>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 7014fe7

NETSTANDARD1_6;
RESHAPED_REFLECTION;
@(DefineConstants);
</DefineConstants>
</PropertyGroup>

<ItemGroup>
<Compile Include="..\paket-files\logary\logary\src\Logary.Facade\Facade.fs" />
<Compile Include="..\Expecto\Performance.fs" />
<Compile Include="..\Expecto\Expecto.fs" />
<Compile Include="..\Expecto\Expect.fs" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.NET.Sdk">
<Version>1.0.0-alpha-20161104-2</Version>
<PrivateAssets>All</PrivateAssets>
</PackageReference>
<PackageReference Include="FSharp.Core">
<Version>4.0.1.7-alpha</Version>
</PackageReference>
<PackageReference Include="Argu">
<Version>3.3.0</Version>
</PackageReference>
<PackageReference Include="System.Linq.Parallel">
<Version>4.3.0</Version>
</PackageReference>
</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard1.6' ">
<PackageReference Include="NETStandard.Library">
<Version>1.6.1</Version>
</PackageReference>
<PackageReference Include="FSharp.NET.Sdk">
<Version>1.0.0-alpha-000009</Version>
</PackageReference>
<PackageReference Include="System.Diagnostics.TraceSource">
<Version>4.3.0</Version>
</PackageReference>
<PackageReference Include="Microsoft.NET.Sdk">
<Version>1.0.0-alpha-20161104-2</Version>
</PackageReference>
</ItemGroup>
<ItemGroup>
<DotNetCliToolReference Include="dotnet-compile-fsc">
<Version>1.0.0-preview2-020000</Version>
</DotNetCliToolReference>
<!-- Allow intellisense https://github.com/dotnet/netcorecli-fsc/wiki/.NET-Core-SDK-rc4#ide-support -->
<PackageReference Include="FSharp.NET.Sdk" Version="1.0.0-beta-060000" PrivateAssets="All" />
<DotNetCliToolReference Include="dotnet-compile-fsc" Version="1.0.0-preview2-020000" />

<PackageReference Include="FSharp.Core" Version="4.1.*" />
Copy link
Owner

Choose a reason for hiding this comment

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

Is there no way to handle this with paket? /cc @forki

Do I now have to change this file and paket.dependencies manually any time any of these change?

Copy link
Contributor

Choose a reason for hiding this comment

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

@haf there is a wip (used in fable-compiler/fable-suave-scaffold ) but ihmo is better merge this pr and after add paket support with another, otherwise you delay a lot because is wip

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with @enricosada - hopefully there will be better solutions in the future. I don't think the burden this introduces (changing version once in a blue moon) is too high for the benefits (support netcore by compiling against netstandard).

Copy link
Owner

Choose a reason for hiding this comment

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

Does this cross-compile against F# 4.0 that Suave.Testing uses? What's the compat there?

Choose a reason for hiding this comment

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

You could do that. Looks like this pull request only targets one framework?

<PackageReference Include="Argu" Version="3.3.0" />
<PackageReference Include="Mono.Cecil" Version="0.10.0-beta2" />
<PackageReference Include="System.Linq.Parallel" Version="4.3.0" />
<PackageReference Include="System.Diagnostics.TraceSource" Version="4.3.0" />
<PackageReference Include="System.Diagnostics.FileVersionInfo" Version="4.3.0" />
</ItemGroup>
<Import Project="$(MSBuildToolsPath)\Microsoft.Common.targets" />
<Import Project="..\.paket\Paket.Restore.targets" />
</Project>
9 changes: 6 additions & 3 deletions Expecto.sln
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@

Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio 14
VisualStudioVersion = 14.0.25420.1
# Visual Studio 15
VisualStudioVersion = 15.0.26206.0
MinimumVisualStudioVersion = 10.0.40219.1
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = ".paket", ".paket", "{3786E131-11FB-4606-AFE0-5E6CDA744DD0}"
ProjectSection(SolutionItems) = preProject
Expand All @@ -18,17 +18,20 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "project", "project", "{1870
ProjectSection(SolutionItems) = preProject
.editorconfig = .editorconfig
.gitignore = .gitignore
.semver = .semver
.travis.yml = .travis.yml
appveyor.yml = appveyor.yml
build.cmd = build.cmd
build.fsx = build.fsx
build.sh = build.sh
DEVGUIDE.md = DEVGUIDE.md
docker-build.sh = docker-build.sh
Gemfile = Gemfile
globals.json = globals.json
global.json = global.json
paket.dependencies = paket.dependencies
Rakefile = Rakefile
README.md = README.md
Settings.FSharpLint = Settings.FSharpLint
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{5B9D0E5F-378A-4310-B9F3-4ECF67AA5A23}"
Expand Down
4 changes: 2 additions & 2 deletions Expecto/Expect.fs
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ let isNotWhitespace (actual : string) format =
let inline equal (actual : 'a) (expected : 'a) format =
match box actual, box expected with
| (:? string as a), (:? string as e) ->
use ai = a.GetEnumerator()
use ei = e.GetEnumerator()
let ai = a.ToCharArray().GetEnumerator()
Copy link
Owner

Choose a reason for hiding this comment

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

Why not use? How do you dispose the enumerator?

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 char[].GetEnumerator() does not implement IDisposable and therefore the use statement cannot be used here. There is no need for disposal because it's all managed GC memory.

let ei = e.ToCharArray().GetEnumerator()
let mutable i = 0
let baseMsg errorIndex =
let diffString = new String(' ', errorIndex + 1) + "↑"
Expand Down
58 changes: 49 additions & 9 deletions Expecto/Expecto.fs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace Expecto
namespace Expecto
Copy link
Owner

Choose a reason for hiding this comment

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

What did you change here?


#nowarn "46"

Expand Down Expand Up @@ -164,8 +164,8 @@ module internal Helpers =

member m.MatchTestsAttributes () =
m.GetCustomAttributes true
|> Array.map (fun t -> t.GetType().FullName)
|> Set.ofArray
|> Seq.map (fun t -> t.GetType().FullName)
|> Set.ofSeq
|> Set.intersect allTestAttributes
|> Set.toList
|> List.choose matchFocusAttributes
Expand Down Expand Up @@ -373,8 +373,7 @@ module Impl =
open Expecto.Logging.Message
open Helpers
open Mono.Cecil
open Mono.Cecil.Rocks


let logger = Log.create "Expecto"

type TestResult =
Expand Down Expand Up @@ -615,14 +614,18 @@ module Impl =
summary = fun _ summary ->
let spirit =
if summary.successful then
#if !NETSTANDARD1_6
Copy link
Owner

Choose a reason for hiding this comment

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

I don't see why this is commented out. According to this issue there's an API to check console output encoding.

if Console.OutputEncoding.BodyName = "utf-8" then
"ᕙ໒( ˵ ಠ ╭͜ʖ╮ ಠೃ ˵ )७ᕗ"
else
#endif
"Success!"
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe is better for maintenance to explicit do #else? so is easier to read

else
#if !NETSTANDARD1_6
Copy link
Owner

Choose a reason for hiding this comment

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

Same here

if Console.OutputEncoding.BodyName = "utf-8" then
"( ರ Ĺ̯ ರೃ )"
else
#endif
""
logger.logWithAck Info (
eventX "EXPECTO! {total} tests run in {duration} – {passes} passed, {ignores} ignored, {failures} failed, {errors} errored. {spirit}"
Expand Down Expand Up @@ -1129,9 +1132,15 @@ module Impl =
let asMembers x = Seq.map (fun m -> m :> MemberInfo) x
let bindingFlags = BindingFlags.Public ||| BindingFlags.Static
fun (t: Type) ->
#if RESHAPED_REFLECTION
Copy link
Owner

Choose a reason for hiding this comment

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

Can't this be used by default, every time? Don't these APIs exist on Mono 4.6.2/.Net 4.6.1 and upwards?

Choose a reason for hiding this comment

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

They do. You have .GetTypeInfo() wrappers for .net 4.5 as well.

Copy link
Owner

Choose a reason for hiding this comment

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

Great, let's try to use them.

[ t.GetTypeInfo().GetMethods bindingFlags |> asMembers
t.GetTypeInfo().GetProperties bindingFlags |> asMembers
t.GetTypeInfo().GetFields bindingFlags |> asMembers ]
#else
[ t.GetMethods bindingFlags |> asMembers
t.GetProperties bindingFlags |> asMembers
t.GetFields bindingFlags |> asMembers ]
#endif
|> Seq.collect id
|> Seq.choose testFromMember
|> Seq.toList
Expand All @@ -1143,6 +1152,16 @@ module Impl =
// might be able to find corresponding source code) is referred to in a field
// of the function object.
let isFsharpFuncType t =
#if RESHAPED_REFLECTION
Copy link
Owner

Choose a reason for hiding this comment

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

Same here

let baseType =
let rec findBase (t:Type) =
if t.GetTypeInfo().BaseType = null || t.GetTypeInfo().BaseType = typeof<obj> then
t
else
findBase (t.GetTypeInfo().BaseType)
findBase t
baseType.GetTypeInfo().IsGenericType && baseType.GetTypeInfo().GetGenericTypeDefinition() = typedefof<FSharpFunc<unit, unit>>
#else
let baseType =
let rec findBase (t:Type) =
if t.BaseType = null || t.BaseType = typeof<obj> then
Expand All @@ -1151,14 +1170,23 @@ module Impl =
findBase t.BaseType
findBase t
baseType.IsGenericType && baseType.GetGenericTypeDefinition() = typedefof<FSharpFunc<unit, unit>>
#endif

let getFuncTypeToUse (testFunc:unit->unit) (asm:Assembly) =
let t = testFunc.GetType()
#if RESHAPED_REFLECTION
if t.GetTypeInfo().Assembly.FullName = asm.FullName then
#else
if t.Assembly.FullName = asm.FullName then
#endif
t
else
let nestedFunc =
#if RESHAPED_REFLECTION

Choose a reason for hiding this comment

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

There shouldn't be a need for this, unless there is some performance issue to call the GetTypeInfo wrappers?

t.GetTypeInfo().GetFields()
#else
t.GetFields()
#endif
|> Seq.tryFind (fun f -> isFsharpFuncType f.FieldType)
match nestedFunc with
| Some f -> f.GetValue(testFunc).GetType()
Expand All @@ -1168,7 +1196,12 @@ module Impl =
match testCode with
| Sync test ->
let t = getFuncTypeToUse test asm
let m = t.GetMethods () |> Seq.find (fun m -> (m.Name = "Invoke") && (m.DeclaringType = t))
#if RESHAPED_REFLECTION
let m = t.GetTypeInfo().GetMethods()
#else
let m = t.GetMethods ()
#endif
|> Seq.find (fun m -> (m.Name = "Invoke") && (m.DeclaringType = t))
(t.FullName, m.Name)
| Async _ | AsyncFsCheck _ ->
("Unknown Async", "Unknown Async")
Expand All @@ -1188,13 +1221,16 @@ module Impl =

let getMethods typeName =
match types.TryFind (getEcma335TypeName typeName) with
| Some t -> Some (t.GetMethods())
| Some t -> Some (t.Methods)
| _ -> None

let getFirstOrDefaultSequencePoint (m:MethodDefinition) =
m.Body.Instructions
|> Seq.tryFind (fun i -> (i.SequencePoint <> null && i.SequencePoint.StartLine <> lineNumberIndicatingHiddenLine))
|> Option.map (fun i -> i.SequencePoint)
|> Seq.tryPick (fun i ->
let sp = m.DebugInformation.GetSequencePoint i
if sp <> null && sp.StartLine <> lineNumberIndicatingHiddenLine then
Some sp else None)
|> Option.map (fun maybeSequencePoint -> maybeSequencePoint)

match getMethods className with
| None -> SourceLocation.empty
Expand Down Expand Up @@ -1407,7 +1443,11 @@ module Tests =
module ExpectoConfig =

let expectoVersion() =
#if RESHAPED_REFLECTION
let assembly = typeof<ExpectoConfig>.GetTypeInfo().Assembly
#else
let assembly = Assembly.GetExecutingAssembly()
#endif
let fileInfoVersion = FileVersionInfo.GetVersionInfo assembly.Location
fileInfoVersion.ProductVersion

Expand Down
Loading