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

Improved ExtractionTest Validation of Attachment and Extracted Children Expected vs Found Count #188

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 46 additions & 8 deletions src/test/java/emissary/test/core/junit5/ExtractionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ public abstract class ExtractionTest extends UnitTest {
private static final List<IBaseDataObject> NO_ATTACHMENTS = Collections.emptyList();
private static final byte[] INCORRECT_VIEW_MESSAGE = "This is the incorrect view, the place should not have processed this view".getBytes();

private boolean skipAttCountValidation = false;
private boolean skipExtractCountValidation = false;

protected KffDataObjectHandler kff =
new KffDataObjectHandler(KffDataObjectHandler.TRUNCATE_KNOWN_DATA, KffDataObjectHandler.SET_FORM_WHEN_KNOWN,
KffDataObjectHandler.SET_FILE_TYPE);
Expand Down Expand Up @@ -78,7 +81,7 @@ public static Stream<? extends Arguments> data() {

/**
* Allow overriding the initial form in extensions to this test.
*
*
* By default, get the initial form from the filename in the form {@code INITIAL_FORM@2.dat} where {@code INITIAL_FORM}
* will be the initial form.
*
Expand Down Expand Up @@ -173,6 +176,18 @@ protected void checkAnswers(Element el, IBaseDataObject payload, @Nullable List<
throws DataConversionException {

int numAtt = JDOMUtil.getChildIntValue(el, "numAttachments");
long numAttElements = el.getChildren().stream().filter(c -> c.getName().startsWith("att")).count();
if (numAtt > -1 || numAttElements > 0) {
if (!skipAttCountValidation) {
assertEquals(Math.max(numAtt, 0), Math.toIntExact(numAttElements),
"Expected numAttachments in " + tname + " not equal to number of <att#> elements");
} else {
if (Math.toIntExact(numAttElements) != numAtt) {
logger.warn("Expected numAttachments and actual <att#> count in {} not equal. Expected: {} Actual: {}", tname,
Math.max(numAtt, 0), numAttElements);
}
}
}
if (numAtt > -1) {
assertEquals(numAtt, attachments != null ? attachments.size() : 0, "Number of attachments in " + tname);
}
Expand Down Expand Up @@ -280,15 +295,29 @@ protected void checkAnswers(Element el, IBaseDataObject payload, @Nullable List<


// Check each extract
String extractCountStr = el.getChildTextTrim("extractCount");
int extractCount = JDOMUtil.getChildIntValue(el, "extractCount");
long numExtractElements =
el.getChildren().stream().filter(c -> c.getName().startsWith("extract") && !c.getName().startsWith("extractCount")).count();
if (extractCount > -1 || numExtractElements > 0) {
if (!skipExtractCountValidation) {
assertEquals(Math.max(extractCount, 0), Math.toIntExact(numExtractElements),
"Expected extractCount in " + tname + " not equal to number of <extract#> elements");
} else {
if (Math.toIntExact(numExtractElements) != extractCount) {
logger.warn("Expected extractCount and actual <extract#> count in {} not equal. Expected: {} Actual: {}", tname,
Math.max(extractCount, 0), numExtractElements);
}
}
}

if (payload.hasExtractedRecords()) {
List<IBaseDataObject> extractedChildren = payload.getExtractedRecords();
int foundCount = extractedChildren.size();

if (extractCountStr != null) {
assertEquals(Integer.parseInt(extractCountStr), foundCount,
String.format("Number of extracted children in '%s' is %s, not expected %s", tname, foundCount, extractCountStr));
if (extractCount > -1) {
assertEquals(extractCount, foundCount,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this get the same skipExtractCountValidation treatment as above? (warn vs assert)

Copy link
Collaborator Author

@sambish5 sambish5 Sep 23, 2024

Choose a reason for hiding this comment

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

I could, though that is kind of out of the scope of this PR. The check above this verifies answer files have matching counts within the file, I'm not sure if we want to have the option to "skip validation" when actually checking the answerFile vs the explicit payload extracted records count

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah - you're 100% right. I lost track of which count the various variables were referencing.

String.format("Number of extracted children in '%s' is %s, not expected %s", tname, foundCount,
extractCount));
}

int attNum = 1;
Expand All @@ -300,9 +329,9 @@ protected void checkAnswers(Element el, IBaseDataObject payload, @Nullable List<
attNum++;
}
} else {
if (extractCountStr != null) {
assertEquals(0, Integer.parseInt(extractCountStr),
String.format("No extracted children in '%s' when expecting %s", tname, extractCountStr));
if (extractCount > -1) {
assertEquals(0, extractCount,
String.format("No extracted children in '%s' when expecting %s", tname, extractCount));
}
}
}
Expand Down Expand Up @@ -419,4 +448,13 @@ protected void setupPayload(IBaseDataObject payload, Document doc) {
payload.setFileType(payload.currentForm());
}
}

// allow the validation of att and ext counts to be skipped/not logged in tests
protected void setAttachmentCountValidation(boolean value) {
this.skipAttCountValidation = !value;
}

protected void setExtractCountValidation(boolean value) {
this.skipExtractCountValidation = !value;
}
}