-
Notifications
You must be signed in to change notification settings - Fork 7
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
CSD02 mandatory tests #3
Conversation
@domachine Do you have a small script that imports the library and validates a file against one test? That would help me to test some edge cases of the implementation. Just from looking at the code I couldn't spot anything obvious... |
Yes, I can throw one together. But if you want, you could add these edge cases to the |
I wouldn't mind if you could do that. I would like to use that in the TC's repo to make sure the example files fail the right tests... |
43b0faa
to
ad30a8f
Compare
Here is a git-patch. Save it on your disk and run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comments.
d975647
to
972109d
Compare
Alright I addressed the issues and rebased the history. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor wording change suggested...
- addresses parts of #4 - add test 6.1.27.11
972109d
to
50f6fcf
Compare
Fixed 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR includes the additional mandatory tests from CSD02.
minimalSecurityAdvisoryDoc.js
andminimalVexDoc.js
invalid #4