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

Validate variable length record detection does not report as expected on collection product #491

Closed
jpl-jengelke opened this issue Apr 27, 2022 · 10 comments
Assignees
Labels
B12.1 bug Something isn't working i&t.issue invalid This doesn't seem right needs:triage s.medium

Comments

@jpl-jengelke
Copy link
Contributor

jpl-jengelke commented Apr 27, 2022

🐛 Describe the bug identified during I&T

Unexpectedly, validate does not report the requisite error, “Validate should report that *.tab files must have fixed-length fields”.

🥼 Related Test Case(s)

B12.1/VAL.6: #390

🔁 : Related issues

#406


➕ Additional Details

📜 To Reproduce

Steps to reproduce the behavior:

  1. Run ‘validate -t collection_calib_freq.xml’
  2. Pass: Ensure that an error is reported that *.tab files must have fixed-length fields.
  3. Fail because it did not report the error on the bad data set.

🕵️ Expected behavior

Error is reported that *.tab files must have fixed-length fields.

📚 Version of Software Used

validate 2.2.0

🩺 Test Data / Additional context

https://github.com/NASA-PDS/validate/files/6970348/validate-tab.tar.gz
https://github.com/NASA-PDS/validate/files/6970349/validate-csv.tar.gz

🏞Screenshots

🖥 System Info

  • OS: Mac OS X Big Sur 11.6.5
  • Browser N/A
  • Version N/A

🦄 Related requirements

⚙️ Engineering Details

@jordanpadams jordanpadams changed the title Validate variable length record detection does not report as expected on test data Validate variable length record detection does not report as expected on collection product Apr 27, 2022
@jordanpadams
Copy link
Member

@jpl-jengelke thanks! this is a bug, but we will not fix this at the moment since I believe it only applies to collection products. Can you confirm this is handled correctly for all other products?

@jpl-jengelke
Copy link
Contributor Author

jpl-jengelke commented Apr 27, 2022

@jordanpadams I tested with validate #423 by modifying the file gbo.ast.alcdef-database_v1.0_20211021_checksum_manifest_v1.0.tab to reduce the size of one value (and removing the file size from gbo.ast.alcdef-database_v1.0_20211021_aip_v1.0.xml to let it run).

validate -R pds4.label -t testdata2/gbo.ast.alcdef-database_v1.0_20211021_aip_v1.0.xml

There were no complaints about fixed-length fields in the *.tab files. I believe it affects all products if that test is valid.

@jordanpadams
Copy link
Member

@jpl-jengelke good catch, but this is actually a requirement that has not been implemented yet. The content validation currently only validates Table_Characters within a subset of File_Area_*. See #229

Here is an example of where this validation works (it should fail per the first line of the .tab being one character short).
table_character_failure.tar.gz

@jordanpadams jordanpadams added the duplicate This issue or pull request already exists label Apr 28, 2022
@jpl-jengelke
Copy link
Contributor Author

@jordanpadams Just to cover the bases for documentation purposes, is there a separate ticket to address this for collection products?

@jordanpadams
Copy link
Member

@jpl-jengelke that should be captured by #229 since a collection product has a File_Area_Inventory but will add that explicitly

@jordanpadams jordanpadams removed the duplicate This issue or pull request already exists label May 16, 2022
@jordanpadams jordanpadams reopened this May 16, 2022
@jordanpadams
Copy link
Member

reopening since #390 was incorrectly closed and original implementation was invalid

@jordanpadams
Copy link
Member

closing this again. looks like this ticket is invalid because validate should not check variable length records in collection inventories. collection inventories are delimited tables, not fixed width tabls

@jordanpadams jordanpadams added invalid This doesn't seem right and removed B13.0 labels May 16, 2022
@rchenatjpl
Copy link
Contributor

I guess you (@jordanpadams) figured out it was a File_Area_Inventory. We do and we must allow that to be a Table_Delimited, but if Dick's arguing that any file with extension .TAB should be fixed width, that's got merit, but there might be hundreds of collection*.tab with variable width.

@rchenatjpl
Copy link
Contributor

So am I still needed to test something?

@jordanpadams
Copy link
Member

@rchenatjpl not at this time.

Additionally, per the other thread on #390 (sorry for jumping back and forth), I'm not sure we can, or should, assume fixed width and delimited at the same time based upon a file name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B12.1 bug Something isn't working i&t.issue invalid This doesn't seem right needs:triage s.medium
Projects
None yet
Development

No branches or pull requests

3 participants