-
Notifications
You must be signed in to change notification settings - Fork 77
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
Use NUnit dotnet core test runner #68
Conversation
Getting source and symbol package as AppVeyor artifacts: https://ci.appveyor.com/project/am11/geoip2-dotnet/build/1.0.91/job/ow5cps4m68j4kd1g/artifacts 😄 |
"frameworks": { | ||
"net45": {}, | ||
"net451": {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@oschwald, this change was made because dotnet-test-nunit doesn't support net45. Besides this is unit test project, so I guess this does not matter much? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I did the same thing for MaxMind.MinFraud.UnitTests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! In MinFraud, we can probably simplify it to:
"dependencies": {
"MaxMind.MinFraud": {
"target": "project"
},
"dotnet-test-nunit": "3.4.0-beta-1",
"MockHttpSigned": "1.3.0-netstandard-alpha3",
- "NETStandard.Library": "1.6.0",
"NUnit": "3.2.1"
},
"testRunner": "nunit",
"frameworks": {
"net451": {},
- "netstandard1.6": {
- "imports": [
- "dnxcore50",
- "netcoreapp1.0",
- "portable-net45+win8"
- ],
+ "netcoreapp1.0": {
+ "imports": "portable-net45+win8",
"dependencies": {
"Microsoft.NETCore.App": {
"version": "1.0.0-*",
"type": "platform"
- },
- "System.Console": "4.0.0"
+ }
}
}
}
Thanks for doing this! I'll take a closer look tomorrow. |
Thanks. TravisCI is experiencing a major outage for macOS builds: https://www.traviscistatus.com/incidents/bwp879gf6bc5. I have verified macOS build locally but it would be good to get all-green CI at some point .. :) |
], | ||
"authors": ["Gregory Oschwald", "Mark Fowler"], | ||
"tags": ["maxmind", "ip", "geoip", "geoip2", "geolocation", "ip2location", "iptolocation", "maxmind", "ipv4", "ipv6"], | ||
"iconUrl": "https://raw.githubusercontent.com/maxmind/GeoIP2-dotnet/master/maxmind-logo.png", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file name case on this is different than the actual file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch; fixed by 91248e3. :)
Looks good to me. One very small comment about the logo file name. |
"" | ||
], | ||
"authors": ["Gregory Oschwald", "Mark Fowler"], | ||
"tags": ["maxmind", "ip", "geoip", "geoip2", "geolocation", "ip2location", "iptolocation", "maxmind", "ipv4", "ipv6"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed ip2location
in the tags. Could you remove this? I believe it is trademarked by a competitor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh, I "inherited" these tags from their NuGet page. For some reason I thought that's applicable here as well. 😛
Removed in 444e982.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries. Do you have the page where that is listed? I should probably remove it there too, if possible.
Edit: Oh, it was on the ip2location page? Yeah, the rest of them are good, just not their name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was in another package name (under tags): https://www.nuget.org/packages/Plarium.Geo/ (not sure why this package is using geoip2
as tag).. If we search GeoIP2 on NuGet we get three results including official package, Plarium.Geo and Plarium.Geo.Client. :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that makes sense. It looks like that uses the GeoIP2 CSVs.
* Update project.json for unit tests. * Add tags and icon URL. * Update appveyor.yml * Other cleanup from maxmind/GeoIP2-dotnet#68
* Update project.json for unit tests. * Add tags and icon URL. * Update appveyor.yml * Other cleanup from maxmind/GeoIP2-dotnet#68
Also: * Update `.travis.yml` to use .NET Core released version. * Update `appveyor.yml` with `dotnet pack` and saving package as build artifact (we can configure `deploy:` to NuGet as well). * Minor branding updates including addition of MaxMind-logo and used in project.json -> `iconUrl`. :) * Add ability to set `mmdb` path via command-line argument and environment variable `MAXMIND_BENCHMARK_DB` in `GeoIP2.Benchmark` runner with fallback to CWD/GeoLite2-City.mmdb. * Add ability to set test directory via environment variable `MAXMIND_TEST_BASE_DIR` in `GeoIP2.UnitTests` and falls back to CWD. * Remove `Program.cs` in `UnitTests` project in favour of dotnet-test-nunit runner.
* Update project.json for unit tests. * Add tags and icon URL. * Update appveyor.yml * Other cleanup from maxmind/GeoIP2-dotnet#68
* Update project.json for unit tests. * Add tags and icon URL. * Update appveyor.yml * Other cleanup from maxmind/GeoIP2-dotnet#68
* Update project.json for unit tests. * Add tags and icon URL. * Update appveyor.yml * Other cleanup from maxmind/GeoIP2-dotnet#68
Thanks! |
* Update project.json for unit tests. * Add tags and icon URL. * Update appveyor.yml * Other cleanup from maxmind/GeoIP2-dotnet#68
* Update project.json for unit tests. * Add tags and icon URL. * Update appveyor.yml * Other cleanup from maxmind/GeoIP2-dotnet#68
Use NUnit dotnet core test runner
Also:
.travis.yml
to use .NET Core released version.appveyor.yml
withdotnet pack
and saving package as buildartifact (we can configure
deploy:
to NuGet as well).project.json ->
iconUrl
. :)mmdb
path via command-line argument andenvironment variable
MAXMIND_BENCHMARK_DB
inGeoIP2.Benchmark
runner with fallback to CWD/GeoLite2-City.mmdb.
MAXMIND_TEST_BASE_DIR
inGeoIP2.UnitTests
and falls back to CWD.Program.cs
inUnitTests
project in favour ofdotnet-test-nunit runner.