diff --git a/.github/workflows/github_server.yml b/.github/workflows/github_server.yml index d4cf9764..fb0ddd31 100644 --- a/.github/workflows/github_server.yml +++ b/.github/workflows/github_server.yml @@ -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 @@ -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 diff --git a/.github/workflows/playground.yml b/.github/workflows/playground.yml index caa4f881..cca4ff6d 100644 --- a/.github/workflows/playground.yml +++ b/.github/workflows/playground.yml @@ -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 @@ -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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 530c8679..32685b88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,13 @@ Format: ## [Unreleased] + + +### 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). @@ -52,7 +59,6 @@ Format: ### 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 @@ -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 diff --git a/docassemble/ALKilnTests/data/sources/observation_steps.feature b/docassemble/ALKilnTests/data/sources/observation_steps.feature index 6afeba9f..69875ffa 100644 --- a/docassemble/ALKilnTests/data/sources/observation_steps.feature +++ b/docassemble/ALKilnTests/data/sources/observation_steps.feature @@ -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" @@ -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 diff --git a/lib/scope.js b/lib/scope.js index 65cf65b2..65279c64 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -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. @@ -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) { @@ -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); } diff --git a/lib/steps.js b/lib/steps.js index 54e2ee96..cd7e83a2 100644 --- a/lib/steps.js +++ b/lib/steps.js @@ -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` }); } diff --git a/package.json b/package.json index 0c34bbe4..a35c539c 100644 --- a/package.json +++ b/package.json @@ -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": {