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

issue 499: not checking table EOL #583

Merged
merged 8 commits into from
Dec 29, 2022
Merged

issue 499: not checking table EOL #583

merged 8 commits into from
Dec 29, 2022

Conversation

al-niessner
Copy link
Contributor

@al-niessner al-niessner commented Dec 23, 2022

🗒️ Summary

Applied two changes. Made a strange failure turn into a pass in file referencing. In table checking, added checking the end of the line in a fixed or formatted table like simply deliminated ones.

⚙️ Test Data and/or Report

$ validate M7_217_044546_N.xml


PDS Validate Tool Report

Configuration:
   Version                       3.1.0-SNAPSHOT
   Date                          2022-12-24T00:33:52Z

Parameters:
   Targets                       [file:/tmp/test/M7_217_044546_N.xml]
   Severity Level                WARNING
   Recurse Directories           true
   File Filters Used             [*.xml, *.XML]
   Data Content Validation       on
   Product Level Validation      on
   Max Errors                    100000
   Registered Contexts File      /home/niessner/Projects/PDS/validate/src/main/resources/util/registered_context_products.json

Product Level Validation Results

  FAIL: file:/tmp/test/M7_217_044546_N.xml
    Begin Content Validation: file:/tmp/test/M7_217_044546_N.dat
      ERROR  [error.table.missing_CRLF]   data object 2, record 1: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 2: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 3: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 4: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 5: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 6: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 7: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 8: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 9: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 10: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 11: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 12: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 13: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 14: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 15: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 16: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 17: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 18: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 19: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 20: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 21: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 22: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 23: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 24: Record does not end in carriage-return line feed.
      ERROR  [error.table.missing_CRLF]   data object 2, record 25: Record does not end in carriage-return line feed.
    End Content Validation: file:/tmp/test/M7_217_044546_N.dat
        1 product validation(s) completed

Summary:

  25 error(s)
  0 warning(s)

  Product Validation Summary:
    0          product(s) passed
    1          product(s) failed
    0          product(s) skipped

  Referential Integrity Check Summary:
    0          check(s) passed
    0          check(s) failed
    0          check(s) skipped

  Message Types:
    25           error.table.missing_CRLF

End of Report
Completed execution in 5208 ms

♻️ Related Issues

#499

@al-niessner al-niessner requested a review from a team as a code owner December 23, 2022 21:50
@al-niessner al-niessner self-assigned this Dec 23, 2022
@al-niessner al-niessner marked this pull request as draft December 23, 2022 21:50
Copy link
Member

@nutjob4life nutjob4life left a comment

Choose a reason for hiding this comment

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

Indentation looks odd but should compile just fine!

Copy link
Contributor Author

@al-niessner al-niessner left a comment

Choose a reason for hiding this comment

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

See below

@al-niessner al-niessner marked this pull request as ready for review December 24, 2022 00:40
@al-niessner
Copy link
Contributor Author

@jordanpadams @nutjob4life @tloubrieu-jpl ready for review

@jordanpadams
Copy link
Member

@al-niessner looks great! can we add a regression test for this bug?

@al-niessner
Copy link
Contributor Author

@jordanpadams wrote:

@al-niessner looks great! can we add a regression test for this bug?

Add a positive and negative check. The negative check is working. The positive is not. It is failing either because I cannot alter the XML correctly or the reader is broken (it is but this may be an additional break that we cannot work with) or both. Do we know if validate works with a valid CRLF EOL table? It would help divide where to work for the solution.

@al-niessner
Copy link
Contributor Author

@jordanpadams

Uhg. The current failure that leads to later failures is that the header does not respect the CRLF. If the table declares CRLF is it for whole table or just the rows?

@al-niessner
Copy link
Contributor Author

al-niessner commented Dec 28, 2022

@jordanpadams @nutjob4life @tloubrieu-jpl

Yay! it was all XML after all -- well I could fix it with the XML but do not know if it is actually the correct fix. Now have a positive and negative check for this bug. Ready for review.

Copy link
Member

@jordanpadams jordanpadams left a comment

Choose a reason for hiding this comment

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

looks great. tested successfully.

@al-niessner I know this is annoying, but to keep everyone on the same page with their IDE, would you mind quickly running through the process the integrate the Google Style Guide into your IDE? I think you are using Eclipse, which would be this article, especially the auto-style updates Eclipse can do whenever you save a file. I know we all have our own style, this just helps provide some consistency across the team so we don't have tons of whitespace updates intermingled with actual code updates.

@jordanpadams jordanpadams merged commit 56fe42b into main Dec 29, 2022
@jordanpadams jordanpadams deleted the issue_499 branch December 29, 2022 16:27
@jordanpadams
Copy link
Member

Uhg. The current failure that leads to later failures is that the header does not respect the CRLF. If the table declares CRLF is it for whole table or just the rows?

@al-niessner looks like you fixed the offset issue, which hopefully answered this question, but for Header data objects, these are defined as some number of bytes with some possible header encoding type, but that is it. validate/pds4-jparser should just read past the said number of bytes and then start reading the table data via the offset value.

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.

3 participants