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

Fix #725, partial filename match for downloading fails #959

Merged
merged 1 commit into from
Nov 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/github_server.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ jobs:
EMPTY_STRING: ""
NORMAL_USER_EMAIL: alkiln@example.com
NORMAL_USER_PASSWORD: User123^
WRONG_EMAIL=wrong_email@example.com
WRONG_PASSWORD=wrong_password
WRONG_EMAIL: wrong_email@example.com
WRONG_PASSWORD: wrong_password

steps:
# Place the root directory in this branch to access
Expand Down Expand Up @@ -160,7 +160,7 @@ jobs:
#### Developer note: You can probably leave the rest out
#### To learn more, see https://assemblyline.suffolklitlab.org/docs/alkiln/writing/#optional-inputs
ALKILN_TAG_EXPRESSION: "${{ env.ALKILN_TAG_EXPRESSION }}"
ALKILN_VERSION: delete
ALKILN_VERSION: download

#### Developer note: Example of making an issue when tests fail
#### that includes the text of the failure output file
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/playground.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ jobs:
NORMAL_USER_PASSWORD: ${{ secrets.USER_NO_PERMISSIONS_PASSWORD }}
NORMAL_USER_API_KEY: ${{ secrets.USER_NO_PERMISSIONS_API_KEY }}
INVALID_API_KEY: invalidAPIkey
WRONG_EMAIL=wrong_email@example.com
WRONG_PASSWORD=wrong_password
WRONG_EMAIL: wrong_email@example.com
WRONG_PASSWORD: wrong_password
EMPTY_STRING: ""
SECRET_VAR1: secret-var1-value
SECRET_VAR2: secret-var2-value
Expand Down Expand Up @@ -81,4 +81,4 @@ jobs:
# want to check up on this.
ALKILN_TAG_EXPRESSION: "${{ env.ALKILN_TAG_EXPRESSION }}"
#### Developer note: You can probably leave this out
ALKILN_VERSION: delete
ALKILN_VERSION: download
9 changes: 8 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,20 @@ Format:

## [Unreleased]

<!-- ## [5.13.2] - 2024-11-21 -->

### Fixed
- Fixes download Step unable to use a partial filename match. The Step needed the whole name, including the extension. See [#725](https://github.com/SuffolkLITLab/ALKiln/issues/725).

## [5.13.1] - 2024-10-23

### Changed

- Moves "expected" status error to only be visible to internal test errors. Closes [#933](https://github.com/SuffolkLITLab/ALKiln/issues/933).

### Fixed

- Detects failed sign in. Closes [#918](https://github.com/SuffolkLITLab/ALKiln/issues/918).
- Updates pdfjs-dist for node v20 and v22. See [#952](https://github.com/SuffolkLITLab/ALKiln/issues/952).

### Internal

Expand All @@ -63,6 +69,7 @@ Format:
- Adds decision docs
- Updated CONTRIBUTING.md
- Added example.env, closes [#374](https://github.com/SuffolkLITLab/ALKiln/issues/374)
- Updates pdfjs-dist for node v20 and v22. See [#952](https://github.com/SuffolkLITLab/ALKiln/issues/952).

## [5.13.0] - 2024-07-11

Expand Down
13 changes: 7 additions & 6 deletions docassemble/ALKilnTests/data/sources/observation_steps.feature
Original file line number Diff line number Diff line change
Expand Up @@ -239,19 +239,19 @@ Scenario: I compare different PDFs
Given the final Scenario status should be "failed"
And the Scenario report should include:
"""
Could not find the existing PDF at DOES_NOT_EXIST.pdf
ALK0156
"""
And the Scenario report should include:
"""
The PDFs were not the same.
ALK0157
"""
And the Scenario report should include:
And the Scenario report should include:
"""
The new PDF added:
- diff
"""
And the Scenario report should include:
"""
- diff
ALK0093
"""
Given I start the interview at "test_pdf"
Then the question id should be "proxy vars"
Expand All @@ -268,6 +268,7 @@ Scenario: I compare different PDFs
And I tap to continue
# Next page
Then the question id should be "2_signature download"
When I download "2_signature.pdf"
# Match a partial name
When I download "2_signatu"
And I expect the baseline PDF "DOES_NOT_EXIST.pdf" and the new PDF "2_signature.pdf" to be the same
And I expect the baseline PDF "linear_2_signature-Baseline.pdf" and the new PDF "2_signature.pdf" to be the same
61 changes: 50 additions & 11 deletions lib/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -2781,7 +2781,7 @@ module.exports = {
await scope.afterStep(scope);
}, // Ends scope.steps.sign()

download: async ( scope, filename ) => {
download: async ( scope, full_or_partial_file_href ) => {
/* Taps the link that leads to the given filename to trigger downloading.
* and waits till the file has been downloaded before allowing the tests to continue.
* WARNING: Cannot download the same file twice in a single scenario.
Expand All @@ -2791,47 +2791,86 @@ module.exports = {
* TODO: Properly wait for download to complete. See notes in
* scope.js scope.detectDownloadComplete()
*/
let [elem] = await scope.page.$$(`xpath/.//a[contains(@href, "${ filename }")]`);
let [elem] = await scope.page.$$(`xpath/.//a[contains(@href, "${ full_or_partial_file_href }")]`);

let msg = `"${ filename }" seems to be missing. Cannot find a link to that document.`;
let msg = `"${ full_or_partial_file_href }" seems to be missing on the page. Cannot find a link to that document.`;
if ( !elem ) { reports.addToReport(scope, { type: `error`, code: `ALK0152`, value: msg }); }
expect( elem, msg ).to.exist;

let failed_to_download = false;
let err_msg = "";
try {
const binaryStr = await scope.page.evaluate(el => {

const { disposition, binaryStr } = await scope.page.evaluate(async function ( el ) {
const url = el.getAttribute("href");
return new Promise(async (resolve, reject) => {
const response = await fetch(url, {method: "GET"});
const reader = new FileReader();
reader.readAsBinaryString(await response.blob());
reader.onload = () => resolve(reader.result);
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition
reader.onload = () => resolve({
disposition: response.headers.get(`Content-Disposition`),
binaryStr: reader.result
});
reader.onerror = () => reject(`🤕 ALK0153 ERROR: Error occurred on page when downloading ${ url }: ${ reader.error }`);
});
}, elem);

err_msg = `could not get the actual name of the file from the response headers.`;
let actual_filename = await scope.steps.get_response_filename({
disposition, default_name: full_or_partial_file_href
});

if (binaryStr !== '') {
err_msg = `binary data for download was empty`;
const fileData = Buffer.from(binaryStr, 'binary');
fs.writeFileSync(`${ scope.paths.scenario }/${ filename }`, fileData);
reports.addToReport(scope, { type: `row info`, code: `ALK0154`, value: `Downloaded ${ filename } (${ fileData.length } bytes) to ${ scope.paths.scenario }`});
fs.writeFileSync(`${ scope.paths.scenario }/${ actual_filename }`, fileData);
reports.addToReport(scope, { type: `row info`, code: `ALK0154`, value: `Downloaded "${ actual_filename }" (${ fileData.length } bytes) to ${ scope.paths.scenario }`});
} else {
failed_to_download = true;
err_msg = `Couldn't download ${ filename }, binary data for download was empty`;
}

} catch (error) {
failed_to_download = true;
err_msg = error;
}

if (failed_to_download) {
reports.addToReport(scope, { type: `warning`, code: `ALK0155`, value: `Could not download file using fetch (${ err_msg }). ALKiln will now fallback to the click download method.` });
scope.toDownload = filename;
reports.addToReport(scope, { type: `warning`, code: `ALK0155`, value: `Could not download a file matching the name "${ full_or_partial_file_href }" using fetch (${ err_msg }). ALKiln will now fallback to the click download method.` });
scope.toDownload = full_or_partial_file_href;
// Should this be using `scope.tapElement`?
await elem.evaluate( elem => { return elem.click(); });
await scope.detectDownloadComplete( scope );
}
}, // Ends scope.steps.download()

get_response_filename: async function({ disposition, default_name=`found_no_file_name.pdf` }) {
/** Given a fetch response headers' Content-Disposition str, return the
* filename of the response's file.
*
* @return { string | null } - Name of fetched file
* */
let filename = default_name;
if ( disposition ) {
filename = disposition.split(`;`)[1].split(`=`)[1];
}

// Handle potential UTF-8 encoded filenames
if ( filename.toLowerCase().startsWith( `utf-8''` )) {
filename = decodeURIComponent( filename.replace( /utf-8''/i, `` ));
} else {
// Replace starting and ending quotes if they exist
filename = filename.replace( /^['"]/, `` ).replace( /['"]$/, `` );
}

// TODO: Add debug log here
// console.log(`filename:`, filename);

// TODO: Add to the report if we had to use the default name

return filename;
}, // Ends scope.steps.get_response_filename()

compare_pdfs: async function (scope, {existing_pdf_path, new_pdf_path}) {
let existing_paths = await scope.findFiles(scope, {to_find_names: [existing_pdf_path]});
if (existing_paths.length == 0) {
Expand All @@ -2852,7 +2891,7 @@ module.exports = {
let removed_str = diffs.filter(part => part.removed).reduce((err_str, part) => {
return err_str + `- ${ part.value }\n`
}, 'The new PDF removed: \n');
let msg = `The PDFs were not the same.\n${ added_str }\n${removed_str}\n\n You can see the full PDFs at ${ full_existing_path } and ${ full_new_pdf_path}`;
let msg = `The PDFs were different.\nAdded:\n${ added_str }\nRemoved:\n${removed_str}\n\nThere might be more information if you actually look at the files. You can see the full PDFs at ${ full_existing_path } and ${ full_new_pdf_path}`;
reports.addToReport(scope, { type: `error`, code: `ALK0157`, value: msg });
scope.failed_pdf_compares.push(msg);
}
Expand Down
3 changes: 2 additions & 1 deletion lib/steps.js
Original file line number Diff line number Diff line change
Expand Up @@ -1285,10 +1285,11 @@ After(async function(scenario) {
if ( scope.failed_pdf_compares.length > 0) {
let msg = scope.failed_pdf_compares.reduce((str, new_msg) => `${ str }\n―――\n${ new_msg }`)
changeable_test_status = `FAILED`;
// TODO: This may be redundant and therefore confusing
reports.addToReport(scope, {
type: `error`,
code: `ALK0093`,
value: `PDF comparison failed ${ scope.failed_pdf_compares.length } time(s)\n―――\n${ msg }\n―――\n`
value: `ALKiln ran into an error when it tried to compare ${ scope.failed_pdf_compares.length } PDF(s)\n―――\n${ msg }\n―――\n`
});
}

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@suffolklitlab/alkiln",
"version": "5.13.1",
"version": "5.13.1-fix-download-name",
"description": "Integrated automated end-to-end testing with docassemble, puppeteer, and cucumber.",
"main": "lib/index.js",
"scripts": {
Expand Down
Loading