Update ABL scripts to use CORGI #2235
Conversation
- Remove old test data - Add new test data - Update snapshots with new script NOTE: The multi-version test data does not include multiple code versions because the CORGI endpoint will support a query argument that filters by code version (returns <= code_version)
Add additional commit for each book in multi-version test When there is more than one commit for a book, the jq filter should select the newest commit In the test data, the newest commit for each book is bbbbbbb. The books in the snapshot should all use this commit.
TylerZeroMaster
force-pushed
the
corgi-abl
branch
from
64922f0 to
65454d7
Compare
script/validate-abl-import/test.bash
Outdated
| @@ -27,8 +27,10 @@ for input_file in "$my_dir"/*.input.json; do | |||
| # Actual test goes here | |||
| # ------------------------- | |||
| json=$(cat "$input_file") | |||
| node "$my_dir/../entry.js" transform-approved-books-data --data "$json" > "$output_file" | |||
| configured_books="{}" | |||
There was a problem hiding this comment.
shouldn't there be tests that exercise the logic of diffing the books against the existing config?
There was a problem hiding this comment.
Thanks for the quick review! Yes, that completely slipped my mind. I added some additional test data for this. The smaller of the new data sets demonstrates what occurs when new versions are present, the other uses some real data to demonstrate that there are no new entries when all ABL versions are already present.
|
|
||
| set -e | ||
|
|
||
| data="$1" |
There was a problem hiding this comment.
i'm not sure it really matters, but this seems like a lot to be putting in the arguments. maybe it could read data from stdin and configured_books from a given file path?
There was a problem hiding this comment.
That is a good point. I originally used inputs like this to separate fetching from transformation and the errors that could occur in each process. I updated the transformation script to read input from 2 files and do some additional checks on the input.
Use slurp to read input from two files into an array
Use process substitution for fetched ABL data Remove unnecessary check for `detail` property (should be handled by --fail option to curl)
Read abl data and configured books from each input file
The data was limited to the first 25 entries for brevity Regardless, the result is the same: no new entries for 20240226.174525 because all the versions from the ABL are already present in configured books
|
i think we should wait to merge this until the current batch of updates is done. @TylerZeroMaster is the new system being used already? are we good to merge this whenever? |
@TomWoodward The new system is in production, but it is not being used yet: there is another PR in progress to update Enki/webhosting pipeline to use the new system as well. We can wait to merge that Enki PR alongside this one. Additionally, if the ABL on Github is updated, the version on CORGI will need to be updated to match. I have a script that will "fast-forward" the CORGI ABL data to match what is present on Github, but I have been running it manually. Overall, the changes can be merged anytime, but I would appreciate a little bit of warning ahead of time. |
|
@TylerZeroMaster ok, we will shoot to merge this maybe next week after the current batch of updates is done. also the "ABL Schema Tests' check is failing with some jq compile error |
In version 1.7, `if` defaults to returning `.` in the absence of an `else` In version 1.6, you must explicitly specify `else .`
Ahh, I think that was because I was using jq version 1.7 which has some changes that must not be backwards compatible. It has been fixed. Let me know when we are ready to merge this change. |
|
@TomWoodward will bypass the required JIRA ticket (CE Tech doesn't use JIRA) and merge this. |
part of openstax/ce#2182
This new script effectively performs the same steps as the old transformation script:
REACT_APP_ARCHIVEREACT_APP_ARCHIVEis read fromsrc/config.archive-url.jsonThe tests show the input and output formats (the output format is the same as before:
{ <uuid>: <commit_sha>, ... })