Embargo script updates #8
Conversation
Merge branch 'embargo-script-update' into embargo-script-updates
…ase when xslt returns text and not html/xml that can be emitted by etree
|
@ctgraham I have done most of the issues now. The remaining issues on pitt qa are
There's also a few cases for works having other works as children that I don't see an example of in default admin set. Otherwise I think the tests are now sufficient to be merged, and for some issues (like issue 379) to be closed on notch8 side. |
ctgraham
left a comment
ctgraham
left a comment
There was a problem hiding this comment.
An incomplete review, but it's been in draft over the weekend, and I wanted to get it out the door.
runtests.py
Outdated
| variables.pop(variable, None) | ||
|
|
||
| if source is None: | ||
| print("ERROR: expected data, but found None") |
There was a problem hiding this comment.
For error messaging, let's send this to the STDERR stream, e.g.:
print("message", file=sys.stderr)
runtests.py
Outdated
|
|
||
| if source is None: | ||
| print("ERROR: expected data, but found None") | ||
| print("Ignoring file writes") |
There was a problem hiding this comment.
Is there any additional context we can provide here regarding implications of this failure? Perhaps the assignments which were skipped?
embargo/embargo.json
Outdated
| "Form": ["payload=files/EtdEmbargo.zip"], | ||
| "Expected":201, | ||
| "Store":["temp/post.body.out=*", | ||
| "WORKID=//*[local-name() = \"id\"]/text()", |
There was a problem hiding this comment.
This is probably just an artifact from the original .sh script, but we don't really want any id or link element, but rather an id and link element from a specific namespace. The .sh does this by querying the namespace-uri() function because xmllint does not support namespace aliases or QNames.
sword-v2-qa/embargo/embargo.sh
Lines 12 to 13 in db36461
We can do this with the atom alias.
| "Method": "XSLT", | ||
| "URI": "embargo/get-first-fileset.xsl", | ||
| "Payload": "temp/post.body.out", | ||
| "Store":["FILESET_URL=//resultroot/*[1]/text()"] |
There was a problem hiding this comment.
This looks like a simpler xpath would be /resultroot/result.
| "URI": "embargo/h4cmeta.xsl", | ||
| "Payload": "temp/post.body.out", | ||
| "Store":["temp/post.metadata.xml=*"], | ||
| "Test": ["embargo=//metadata/visibility/text()", |
There was a problem hiding this comment.
Each of these would be better with a root match (/) rather than a global search (//),
| import csv | ||
| import sys | ||
|
|
||
| csv_headers = ("Title","Method","URI","Headers", |
There was a problem hiding this comment.
Why are the CSV headers named here, but also in the YAMLs as a Headers entry?
Created embargo.csv in embargo folder as well as a utility to allow creating csv testfiles from json