sillsdev / machine

Machine is a natural language processing library for .NET that is focused on providing tools for processing resource-poor languages.
MIT License
26 stars 15 forks source link

Test coverage #182

Closed johnml1135 closed 6 months ago

johnml1135 commented 6 months ago

https://github.com/sillsdev/machine/issues/181


This change is Reviewable

codecov-commenter commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 67.09%. Comparing base (f1fcf56) to head (a19b9d2).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #182 +/- ## ========================================== + Coverage 67.04% 67.09% +0.05% ========================================== Files 441 441 Lines 34831 34826 -5 Branches 4670 4669 -1 ========================================== + Hits 23351 23368 +17 + Misses 10387 10365 -22 Partials 1093 1093 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

johnml1135 commented 6 months ago

tests/SIL.Machine.AspNetCore.Tests/SIL.Machine.AspNetCore.Tests.csproj line 16 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…
What is the difference between this package and `coverlet.collector`?

This can be run locally to assess coverage. It helps me when I write tests to verify that the coverage I expect is the coverage that I get.

johnml1135 commented 6 months ago

tests/SIL.Machine.Tests/Corpora/ScriptureRefTests.cs line 55 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…
I don't quite understand what this test is buying us. It seems to be mostly testing `List.Sort` and `HashSet`.

this tests the IComparable.CompareTo method

johnml1135 commented 6 months ago

tests/SIL.Machine.Tests/Corpora/ScriptureRefTests.cs line 78 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…
I don't think `Overlaps` is being used, so I wanted to remove it.

Removed.

johnml1135 commented 6 months ago

tests/SIL.Machine.Tests/Corpora/ScriptureRefTests.cs line 99 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…
This should be a separate test.

Done

johnml1135 commented 6 months ago

tests/SIL.Machine.Tests/Corpora/ScriptureRefTests.cs line 55 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…
We are already testing the `CompareTo` method. We can just remove these tests.

Yes for public int CompareTo(object obj), no for int IComparable<ScriptureRef>.CompareTo(ScriptureRef other). It is just a little thing, but I wanted to make sure it was covered.

johnml1135 commented 6 months ago

tests/SIL.Machine.Tests/Corpora/ScriptureRefTests.cs line 55 at r1 (raw file):

Previously, ddaspit (Damien Daspit) wrote…
If you want to test that method, then test it directly. I think it is unnecessary and only creates more tests that need to be maintained for almost no benefit. You can test it directly by referencing the instance using the interface, i.e. ``` int result = ((IComparable)ref1).CompareTo(ref2); ```

Ah, I was trying to access it directly and didn't know the syntax. thanks! I'll update the the test.