NightOwl888 commented on code in PR #1052:
URL: https://github.com/apache/lucenenet/pull/1052#discussion_r1867054858
##########
src/Lucene.Net.Tests/Util/TestOfflineSorter.cs:
##########
@@ -254,7 +256,9 @@ private void AssertFilesIdentical(FileStream golden,
FileStream sorted)
Stream is2 = sorted;
while ((len = is1.Read(buf1, 0, buf1.Length)) > 0)
{
- is2.Read(buf2, 0, len);
+ var numRead = is2.Read(buf2, 0, len); // LUCENENET specific -
asserting that we read the entire buffer
+ Assert.AreEqual(len, numRead);
Review Comment:
See my comment in IndexAndTaxonomyRevisionTest.
##########
src/Lucene.Net.Tests.Replicator/IndexInputStreamTest.cs:
##########
@@ -55,7 +55,11 @@ public void
Read_RemainingIndexInputLargerThanReadCount_ReturnsExpectedSection([
int readBytes = 1.KiloBytes();
byte[] readBuffer = new byte[readBytes];
for (int i = section; i > 0; i--)
- stream.Read(readBuffer, 0, readBytes);
+ {
+ var numRead = stream.Read(readBuffer, 0, readBytes); //
LUCENENET specific - asserting that we read the entire buffer
+ Assert.AreEqual(readBytes, numRead);
Review Comment:
See my comment in IndexAndTaxonomyRevisionTest.
##########
src/Lucene.Net.Tests/Util/TestOfflineSorter.cs:
##########
@@ -231,7 +231,9 @@ private void AssertFilesIdentical(FileInfo golden, FileInfo
sorted)
using Stream is2 = sorted.Open(FileMode.Open, FileAccess.Read,
FileShare.Delete);
while ((len = is1.Read(buf1, 0, buf1.Length)) > 0)
{
- is2.Read(buf2, 0, len);
+ var numRead = is2.Read(buf2, 0, len); // LUCENENET specific -
asserting that we read the entire buffer
+ Assert.AreEqual(len, numRead);
Review Comment:
See my comment in IndexAndTaxonomyRevisionTest.
##########
src/Lucene.Net.Tests.Replicator/IndexAndTaxonomyRevisionTest.cs:
##########
@@ -178,7 +178,10 @@ public void TestOpen()
offset = skip;
}
src.ReadBytes(srcBytes, offset, srcBytes.Length -
offset);
- @in.Read(inBytes, offset, inBytes.Length - offset);
+
+ var numRead = @in.Read(inBytes, offset, inBytes.Length
- offset); // LUCENENET specific - asserting that we read the entire buffer
+ assertEquals(inBytes.Length - offset, numRead);
Review Comment:
We are already doing a length check on line 165, so there is no need to do
it again here.
`ReadExactly()` can potentially throw an exception if the length passed in
is larger than the number of bytes left to read. This means that any test that
does not assert the length prior to the call to `ReadExactly()` could
potentially throw a less useful exception, which is why I pointed it out in
that other example (although, after reviewing again, I see that the length of
one side is used to set the length of the other, so the assert would never
fail). `ReadExactly()` could work in this case because the prior length check
is being done, thus heading off the exception.
That being said, `ReadExactly()` doesn't seem all that useful unless there
are production cases where the exception would be helpful. In tests, it would
be more practical to suppress the warning to keep this in sync with upstream.
##########
src/Lucene.Net.Tests.Replicator/IndexRevisionTest.cs:
##########
@@ -160,7 +160,10 @@ public void TestOpen()
offset = skip;
}
src.ReadBytes(srcBytes, offset, srcBytes.Length - offset);
- @in.Read(inBytes, offset, inBytes.Length - offset);
+
+ var numRead = @in.Read(inBytes, offset, inBytes.Length -
offset); // LUCENENET specific - asserting that we read the entire buffer
+ assertEquals(inBytes.Length - offset, numRead);
Review Comment:
See my comment in IndexAndTaxonomyRevisionTest.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
