make Feature.properties optional

[janusw]

• up to now it was required, but could be null
• see https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_Required.htm
• this should fix issue JsonSerializationException: 'Required property 'properties' not found in JSON #131

[janusw]

@matt-lethargic @xfischer ping!

[xfischer]

@janusw ping acknowledged. Can you add tests to this PR to make sure a Feature with properties and without properties is properly handled ?

[janusw]

@janusw ping acknowledged.

Thanks for the reply 😆

Can you add tests to this PR to make sure a Feature with properties and without properties is properly handled ?

Yeah, good point. I see that there is a GeoJSON.Net.Tests project with some NUnit integration. However I haven't really managed to execute it. If I try to run the tests in VS4Mac (v17.6.7), I get:

NUnit Adapter 3.8.0.0: Test execution started
Running all tests in ∼/GeoJSON.Net/src/GeoJSON.Net.Tests/bin/Debug/net45/GeoJSON.Net.Tests.dll
Dependent Assembly ∼/GeoJSON.Net/src/GeoJSON.Net.Tests/bin/Debug/net45/GeoJSON.Net.Tests.dll.mdb
of ∼/GeoJSON.Net/src/GeoJSON.Net.Tests/bin/Debug/net45/GeoJSON.Net.Tests.dll not found.
Can be ignored if not a NUnit project.
NUnit Adapter 3.8.0.0: Test execution complete
No test is available in ∼/GeoJSON.Net/src/GeoJSON.Net.Tests/bin/Debug/net45/GeoJSON.Net.Tests.dll.
Make sure that test discoverer & executors are registered and platform & framework version settings are appropriate and try again.

I also tried on the command line (on MacOS), but this gives me:

% dotnet test src/GeoJSON.Net.Tests                                      
  Determining projects to restore...
  All projects are up-to-date for restore.
CSC : error CS7027: Error signing output with public key from file 'key.snk' -- File not found. [∼/GeoJSON.Net/src/GeoJSON.Net/GeoJSON.Net.csproj::TargetFramework=net45]
CSC : error CS7027: Error signing output with public key from file 'key.snk' -- File not found. [∼/GeoJSON.Net/src/GeoJSON.Net/GeoJSON.Net.csproj::TargetFramework=netstandard2.1]

Any advice? What am I doing wrong? How do you run the tests, @xfischer?

[xfischer]

Mmmh, I don't have a mac hanging around, so I cannot reproduce. I'll test on Windows

[janusw]

Also it would be nice to have some CI to do automatic builds and tests for every PR. I see that you have some Azure-based CI on master, but I cannot access this, and it doesn't seem to run on the PRs.

Would you accept a GitHub-Actions CI setup, if I'd contribute it? Or do you prefer to stick to Azure? (One can also have both in parallel.)

[xfischer]

Also it would be nice to have some CI to do automatic builds and tests for every PR. I see that you have some Azure-based CI on master, but I cannot access this, and it doesn't seem to run on the PRs.

Would you accept a GitHub-Actions CI setup, if I'd contribute it? Or do you prefer to stick to Azure? (One can also have both in parallel.)

Yes, that would be great ! @matt-lethargic is it OK for you ?

[janusw]

Actually it seems there is some draft GHA setup already:

https://github.com/GeoJSON-Net/GeoJSON.Net/blob/master/.github/workflows/main.yml

However, it would currently only run on a hypothetic 'v2' branch (which does not exist). I can take that as a starting point and create a working setup from there ...

[janusw]

I also tried on the command line (on MacOS), but this gives me:

% dotnet test src/GeoJSON.Net.Tests                                      
  Determining projects to restore...
  All projects are up-to-date for restore.
CSC : error CS7027: Error signing output with public key from file 'key.snk' -- File not found. [∼/GeoJSON.Net/src/GeoJSON.Net/GeoJSON.Net.csproj::TargetFramework=net45]
CSC : error CS7027: Error signing output with public key from file 'key.snk' -- File not found. [∼/GeoJSON.Net/src/GeoJSON.Net/GeoJSON.Net.csproj::TargetFramework=netstandard2.1]

Actually testing the full sln seems to work well! 🥳

% dotnet test src/GeoJSON.Net.sln
[...]
Test run for ∼/GeoJSON.Net/src/GeoJSON.Net.Tests/bin/Debug/netcoreapp3.1/GeoJSON.Net.Tests.dll (.NETCoreApp,Version=v3.1)
Microsoft (R) Test Execution Command Line Tool Version 17.8.0 (arm64)
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
  Skipped FeatureCollection_Test_IndexOf [42 ms]

Passed!  - Failed:     0, Passed:   120, Skipped:     0, Total:   120, Duration: 1 s - GeoJSON.Net.Tests.dll (netcoreapp3.1)

[janusw]

Can you add tests to this PR to make sure a Feature with properties and without properties is properly handled ?

By now I added a small test case that verifies that a feature without any properties is deserialized alright. Is the PR ok with this addition?

[janusw]

Can you add tests to this PR to make sure a Feature with properties and without properties is properly handled ?

By now I added a small test case that verifies that a feature without any properties is deserialized alright.

... and GitHub Actions verified that it works well and all tests succeed.

Is the PR ok with this addition?

@matt-lethargic @xfischer Ping!

[matt-lethargic]

More than happy to move to GH Actions, the setup we had pre-dates them by a lot.
We have building, but really we should have a workflow that gets kicked off whenever we create a release in GH that pushes out to Nuget

[janusw]

More than happy to move to GH Actions, the setup we had pre-dates them by a lot. We have building, but really we should have a workflow that gets kicked off whenever we create a release in GH that pushes out to Nuget

IMHO the most important part would be to build a nupkg in every GHA build, and make it available as an artifact. That should be pretty simple, and I can take care of it.

Once that is done, pushing such a pkg to nuget.org (for a release tag) is a relatively small step that is easy to do manually But, yes, one can also automate that.

In any case, all of this is orthogonal to this PR, which contains a simple one-line fix (with a simple test case).

[janusw]

More than happy to move to GH Actions, the setup we had pre-dates them by a lot. We have building, but really we should have a workflow that gets kicked off whenever we create a release in GH that pushes out to Nuget

IMHO the most important part would be to build a nupkg in every GHA build, and make it available as an artifact. That should be pretty simple, and I can take care of it.

See PR #175.