Resolve Main Schema URI used for Validation #1358
Conversation
olabusayoT
force-pushed
the
daf-2950-resolveURIForValidation
branch
from
c11f8d3
to
4ed482e
Compare
- currently we pass it in directly without trying to resolve it, which was ok in 3.8.0 because the URI got resolved before we ever got to validation, so we call resolveSchemaLocation on the uriForLoading so that Xerces Validation can use that. - added tests to verify DAFFODIL-2950
olabusayoT
force-pushed
the
daf-2950-resolveURIForValidation
branch
from
4ed482e
to
578a204
Compare
| fail(); | ||
| } | ||
| } catch (Exception e) { | ||
| assertTrue(e.getMessage().contains("Could not find file or resource")); |
There was a problem hiding this comment.
Is this supposed to fail? Looks to me like testJavaAPI.schema should exist, so compileSource and onPath should succeed. Feels like this catch block wants to just contain fail();, or alternatively just remove the try/catch so that any thrown exceptions will cause the test to fail. Maybe we just need a finally block to delete the tmp file?
There was a problem hiding this comment.
So this test is intended to test the case where compileSource succeeds, but onPath can't find the file when it tries to resolve the schemaLocation. It's mainly a coverage test. That's also why we don't call withValidationMode(Full) here, since onPath is where we do the resolution
There was a problem hiding this comment.
I see, can you add a comment to that affect? Also, in that case I think this needs a fail() and the end of the try block in case the exception isn't thrown. It might be a bit cleaner to rewrite it like this:
try {
assertFalse(pf.isError)
// delete file needed by Xerces for full validation
tempFile.delete()
// should throw FileNotFoundException because ...
pf.onPath("/")
// fail if exception was not thrown
fail()
} catch (Exception e) {
assertTrue(e.getMessage().contains(...))
} finally {
if (tempFile.exists) tempFile.delete()
}Removes the conditionals to make it a bit easier to see what's expected.
| try { | ||
| if (!pf.isError()) { | ||
| tempFile.delete(); | ||
| pf.onPath("/"); |
There was a problem hiding this comment.
Should this have withValidationMode(full) so that we make sure full validation mode works when the URI has a file scheme? I assume that's what this test is checking for?
There was a problem hiding this comment.
the withValidationMode(Full) case is already tested in the first CompileSource test
| fail(); | ||
| } | ||
| } catch (Exception e) { | ||
| assertTrue(e.getMessage().contains("Could not find file or resource")); |
There was a problem hiding this comment.
I see, can you add a comment to that affect? Also, in that case I think this needs a fail() and the end of the try block in case the exception isn't thrown. It might be a bit cleaner to rewrite it like this:
try {
assertFalse(pf.isError)
// delete file needed by Xerces for full validation
tempFile.delete()
// should throw FileNotFoundException because ...
pf.onPath("/")
// fail if exception was not thrown
fail()
} catch (Exception e) {
assertTrue(e.getMessage().contains(...))
} finally {
if (tempFile.exists) tempFile.delete()
}Removes the conditionals to make it a bit easier to see what's expected.
| import java.io.ByteArrayOutputStream; | ||
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.io.*; |
There was a problem hiding this comment.
I think we generally try to avoid these star imports.
- fix * in import for java tests - refactor test code and add comments to make purpose more clear - rename Scala test that had Java in the name DAFFODIL-2950
DAFFODIL-2950