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

Add validation to DataObj instantiation #152

Open
bskinn opened this issue Nov 10, 2020 · 0 comments
Open

Add validation to DataObj instantiation #152

bskinn opened this issue Nov 10, 2020 · 0 comments
Labels
area: api 🤖 issue: planned ⌚ Assigned to a specific version's milestone topic: validation 🔎 type: enhancement ✨ Something to add
Milestone

Comments

@bskinn
Copy link
Owner

bskinn commented Nov 10, 2020

Values must not be zero-length, and must not contain whitespace characters. Constraints vary per-field; see syntax.rst.

Per planned fix to #147, all constructor arguments, including priority (but excluding dispname), should hold to this. No specific/special (viz., numeric-only) validation on priority.

This is in actuality a breaking change to the DataObjStr and DataObjBytes instantiation behavior, even though it's ultimately adding a guard against the user making a DataObj and/or Inventory that will be broken for downstream use. Best to save it for a major version bump.

Should probably implement a safety hatch for users who really want to make invalid objects, in case this validation is wrong/incorrect or irrelevant to an unanticipated use case. Simplest approach is to have a flag to ignore errors? Maybe convert them into warnings?

Would probably be a good opportunity to try adding in hypothesis -- it shouldn't be too hard to define coherent properties to apply to each field.

Once the validation is in place, the valid-objects tests won't work properly any more. There will still be expect-valid and expect-invalid cases; just, the check will have to be implemented differently. For expected-valid, the dos instantiation should work, and the object should appear in the IFile.load() contents. For expected-invalid, the dos instantiation should fail, and the object should not appear in the IFile.load() contents.

@bskinn bskinn modified the milestones: v3.0, v2.1 Nov 10, 2020
bskinn added a commit that referenced this issue Feb 13, 2021
Adding some hypothesis tests and DataObj validation (#152) will
further strengthen confidence around (and possibly obviate)
these tests.
@bskinn bskinn added issue: planned ⌚ Assigned to a specific version's milestone topic: validation 🔎 and removed type: bug 🪲 Something is wrong pr: needs tests ✔️ labels Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: api 🤖 issue: planned ⌚ Assigned to a specific version's milestone topic: validation 🔎 type: enhancement ✨ Something to add
Projects
None yet
Development

No branches or pull requests

1 participant