stevedlawrence commented on a change in pull request #748:
URL: https://github.com/apache/daffodil/pull/748#discussion_r802633170
##########
File path:
daffodil-io/src/test/scala/org/apache/daffodil/io/TestDataOutputStream4.scala
##########
@@ -41,10 +41,10 @@ class TestDataOutputStream4 {
val out = direct.addBuffered
if (setAbs)
- out.setAbsStartingBitPos0b(ULong(20))
+ out.setAbsStartingBitPos0b(ULong(19))
Review comment:
These tests definitely run, that's how I found that they had this
off-by-one bug. I think the issue was that these tests just didn't do anything
that required a correct absolution starting position. When this tests makes the
buffers final and we deliver the buffered content, we just concatenate the
buffered content to the direct buffer--we don't actually care where the buffer
content starts. And these tests only verify that the content is correct--it
doesn't look at any other state of the DOS.
I think the only things that really require correct abs bit positions is
things like alignment unparsers, length calculations, etc. none of which exist
in this small unit test.
And I think all our other DOS logic in Daffodil proper is correct and so we
never ran into issues where the starting absolute bit position was actually
used (e.g. alignment, length calculations, etc.). This new assertion I added
verifies that.
--
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]
