Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

provide more info when SBCF are different #771

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rupert-griffin
Copy link
Collaborator

@rupert-griffin rupert-griffin commented May 8, 2024

When two SeekableByteChannelFactory objects are found to not be the same, the differences string is currently appended with "<data parameter name> not equal. 1.cs=<byte channel 1 size> 2.cs=<byte channel 2 size>".

This doesn't account for channels of the same size with different data. Finding these differences currently means stepping through the program with the debugger to manually convert each channel's input stream back into a string. Appending the data directly from each object makes this issue much easier to debug.

differences was also removed as the error message for assertNull in RegressionTestUtil because it already prints when the assert fails.

@rupert-griffin rupert-griffin marked this pull request as draft May 8, 2024 20:08
@@ -157,8 +158,10 @@ public static void diff(final SeekableByteChannelFactory sbcf1, final SeekableBy
InputStream is1 = Channels.newInputStream(sbc1);
InputStream is2 = Channels.newInputStream(sbc2)) {
if (!IOUtils.contentEquals(is1, is2)) {
differences.add(String.format("%s not equal. 1.cs=%s 2.cs=%s",
identifier, sbc1.size(), sbc2.size()));
differences.add(String.format("%s not equal. 1.is=%s 2.is=%s",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely don't want to spit out a string representation of the entire payload; that could be well over a GB for each object being compared.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense, I hadn't considered that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to have it display a string representation of the first x characters? Where x is set to 64 for ease of debugging

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could consider making that an option, but it would only be helpful if 1) the payload is text-like and decodes well using UTF_8 2) the difference occurs within the first few bytes.

@@ -121,7 +122,8 @@ public static void diff(final List<IBaseDataObject> ibdoList1, final List<IBaseD
final int ibdoList2Size = (ibdoList2 == null) ? 0 : ibdoList2.size();

if (ibdoList1Size != ibdoList2Size) {
differences.add(String.format("%s%s: 1.s=%s 2.s=%s", identifier, ARE_NOT_EQUAL, ibdoList1Size, ibdoList2Size));
differences.add(String.format("%s%s%sIBaseDataObject List Size 1: %s%sIBaseDataObject List Size 2: %s",
identifier, ARE_NOT_EQUAL, StringUtils.LF, ibdoList1Size, StringUtils.LF, ibdoList2Size));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we insert line breaks, it will break parsing of aggregated messages.

@rupert-griffin
Copy link
Collaborator Author

rupert-griffin commented May 15, 2024

Line breaks and payload string removed from log messaging. Returned to original formatting with slightly adjusted wording for improved clarity.

Before:
[ERROR] ExamplePlaceTest>RegressionTest.testExtractionPlace:266->ExtractionTest.testExtractionPlace:114->RegressionTest.checkAnswers:337 com.example.transform.ExamplePlace: PDiff: data not equal. 1.cs=64 2.cs=64 ==> expected: <null> but was: <com.example.transform.FfmpegPlace: PDiff: data not equal. 1.cs=64 2.cs=64>

After:
[ERROR] ExamplePlaceTest>RegressionTest.testExtractionPlace:266->ExtractionTest.testExtractionPlace:114->RegressionTest.checkAnswers:337 expected: <null> but was: <com.example.transform.ExamplePlace: parent_differences: <data> elements are not equal. SeekableByteChannelFactory_1=64_byte_stream : SeekableByteChannelFactory_2=64_byte_stream >

@rupert-griffin rupert-griffin marked this pull request as ready for review May 17, 2024 18:17
@jpdahlke jpdahlke added this to the v8.4.0 milestone May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants