Test coverage

[johnml1135]

#181

---

This change is 

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files

@@            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              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ddaspit]

Reviewed 10 of 10 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Enkidu93 and @johnml1135)

---

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

	<ItemGroup>
		<PackageReference Include="coverlet.msbuild" Version="6.0.2">

What is the difference between this package and coverlet.collector?

---

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

    [TestCase]
    public void CompareTemplate()

I don't quite understand what this test is buying us. It seems to be mostly testing List.Sort and HashSet.

---

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

    [TestCase("MAT 1:0/1:esb", "MAT 1:0/1:esb/1:p", ExpectedResult = false, Description = "NonVerseParentChild")]
    [TestCase("MAT 1:0/1:p/2:esb", "MAT 1:0/2:esb/1:p", ExpectedResult = false, Description = "NonVerseParentChild")]
    public bool Overlaps(string ref1Str, string ref2Str)

I don't think Overlaps is being used, so I wanted to remove it.

---

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

            Assert.That(!ref1.Equals(obj1));
        });
        Assert.Throws<ArgumentException>(() => ref1.CompareTo(obj1));

This should be a separate test.

[johnml1135]

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]

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]

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]

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

[ddaspit]

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Enkidu93 and @johnml1135)

---

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

Previously, johnml1135 (John Lambert) wrote…

this tests the IComparable.CompareTo method

We are already testing the CompareTo method. We can just remove these tests.

[johnml1135]

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.

[ddaspit]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Enkidu93 and @johnml1135)

---

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

Previously, johnml1135 (John Lambert) wrote…

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.

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<ScriptureRef>)ref1).CompareTo(ref2);

[johnml1135]

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<ScriptureRef>)ref1).CompareTo(ref2);

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

[ddaspit]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Enkidu93)

[ddaspit]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @johnml1135)