Reactive: Increase test coverage and the amount of platforms used during continuous integration validation

[ghuntley]

We need to make it more accessible and safer for folks to contribute to this project. I would love to do a performance pass but not before basics such as increasing test coverage happen.

So this is a help wanted? Can you help out? Will you.

Deliverables:

• Determine current coverage
• Increase test coverage where valuable and needed.
• Execute validation tests on Android & iOS platforms using https://github.com/xamarin/Essentials/tree/master/DeviceTests

[danielcweber]

I can check code coverage and evaluate what's missing.

[ghuntley]

🏆 Make it so.

[danielcweber]

The code coverage is just awesome. We have a coverage of 98% for the .netcore 2.0 run of the the tests. I did some updates though and will try to get rid of the nasty file-linking approach that is currently in place to enable the same unit tests on multiple platforms. I should be able to add a test runner for Xamarin.Android, every other platform is out of my scope unfortunately.

[danielcweber]

A test runner has been added (#484). Removing file linking would not work unfortunately because xUnit would just not discover tests in referenced assemblies, so the new Android app will use file linking as well. This is a potential source for omissions. Maybe the files could be linked by wildcards?

[danielcweber]

@onovotny Can we quickly and easily re-enable coveralls reporting by changing this ?

[clairernovotny]

That should be handled already by
https://github.com/dotnet/reactive/blob/f94b0062606c1f69be5c56594e016110c68f6209/Rx.NET/Source/build-new.ps1#L5

[clairernovotny]

I think it just needs COVERALLS_REPO_TOKEN set...

[clairernovotny]

Okay, coverage is showing up now for CI builds:

https://coveralls.io/github/dotnet/reactive

[akarnokd]

Looks like the source is not linked properly yet: https://coveralls.io/builds/17127097/source?filename=src%2FSystem.Reactive%2FLinq%2FObservable%2FAsObservable.cs

[clairernovotny]

should be fixed now

[akarnokd]

Thanks, it works now!

Btw, I've updated a PR and the build says: Blank or invalid Coveralls Repo Token.

I'm assuming this is to avoid leaking the token but then how can a PR be assessed of its coverage?

[clairernovotny]

That's a good question. There's no way to keep secrets on PRs....

[akarnokd]

Yes. I guess until running code coverage on .NET code becomes free just like in the Java world, we may only rely on follow-up PRs addressing the lack of reasonable coverage on new/changed components.

[clairernovotny]

Running it is free, reporting the results needs somewhere to report it. If you run the build script locally, it renders a code coverage result.

[clairernovotny]

Once Code Coverage with VS Test works solidly with .NET Core (there are still issues), then I think it'll be integrated into VSTS and it may be possible to fail a build that way...or do something else meaningful.

[akarnokd]

I think this has been addressed.