diff --git a/.github/workflows/github_server.yml b/.github/workflows/github_server.yml index 20a59b35..f5ee7175 100644 --- a/.github/workflows/github_server.yml +++ b/.github/workflows/github_server.yml @@ -157,7 +157,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: + ALKILN_VERSION: "screenshot" #### 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 73089c6e..ee4e958c 100644 --- a/.github/workflows/playground.yml +++ b/.github/workflows/playground.yml @@ -80,4 +80,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: + ALKILN_VERSION: "screenshot" diff --git a/CHANGELOG.md b/CHANGELOG.md index a23aa7b7..c5b68cd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,16 +47,21 @@ Format: ### Changed +- On pages with sensitive answers, store the HTML of the page. The HTML excludes field values, so those sensitive answers will not be in the saved file. Still avoid taking a pic of the screen, which would reveal sensitive answers. ‼️ NEVER USE REAL USERS' ANSWERS IN ALKILN TESTS. This HTML can still reveal information about a user's answers. For example, some answers will reveal new questions. That will change the code of the revealed fields and that code will be in the HTML. See #1099 - GitHub action release: Restored our GitHub action's default for ALKiln version to the latest version 5 again. Released first on GitHub actions. NPM release will come in time, but npm has no impact on GitHub action releases. ### Fixed -- Fixed GitHub+You action outdated docassemble cli version causing Scenario timeouts. +- Updated docassemble cli version. +- Better detect navigation when email and password fail to sign in to a docassemble server account. ### Internal -- Simplify workflow files by removing `node` env setup (leaving it to the actions) +- Broke out the function in `Before()` and `After()` to attempt better error tracing. Goal: try to repeat this for other functions in that file. +- Simplified workflow files by removing `node` env setup (leaving it to the actions) - Updated both of our actions' dependencies (the checkout, setup-node, setup-python, upload-artifacts, download-artifacts actions). Closes [#1095](https://github.com/suffolkLITLab/aLKiln/issues/1095). Once again, action related. +- When our report phrases are missing, list all missing phrases at one time in our custom message. The cucumber message will stay the same. +- Silenced errors from screenshots & HTML downloads when trying to provide more information about failing tests. Those records are nice to have, but not absolutely necessary, and if a server is busy puppeteer will rightly have lost execution context, preventing interactions with the page and we don't need a noisy error about it. Log any problems. ## [5.16.1] - 2026-06-01 diff --git a/lib/scope.js b/lib/scope.js index 8a586c67..f7a78321 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -2711,8 +2711,23 @@ module.exports = { return scope; }, // Ends scope.throwPageError() - take_a_screenshot: async ( scope,{ path }) => { - /* Takes a jpeg screenshot. Avoids destroying signatures. */ + take_a_screenshot: async ( scope, { + path, disable_pic=false, disable_html=false + }) => { + /** + * Saves a jpeg screenshot and page HTML to `path`. Avoids destroying + * signatures. + * + * @param {obj} scope - State and internal functions + * @param {obj} obj - Named arguments + * @param {str} obj.path - name of path where to save the files + * @param {bool} [obj.disable_pic=false] - Optional. Whether to disable a + * puppeteer screenshot + * @param {bool} [obj.disable_html=false] - Optional. Whether to disable + * saving the HTML of the page + * + * @returns undefined + * */ let fullPage = true; let signature_elem = await scope.page.$(scope.signature_selector); @@ -2721,28 +2736,32 @@ module.exports = { fullPage = false; } - await scope.page.screenshot({ - path: path, - type: 'jpeg', - fullPage: fullPage, - }); - - let html_path = path; - if (path.endsWith(".jpg")) { - html_path = html_path.substring(0, html_path.length - 4) + ".html"; - } else { - html_path = html_path + ".html"; + if ( !disable_pic ){ + await scope.page.screenshot({ + path: path, + type: 'jpeg', + fullPage: fullPage, + }); } - // Also save the HTML of the page - await scope.page.content().then(content => { - let server_url = session_vars.get_da_server_url(); - content = content.replaceAll(/"(\/static\/.*\.css\?v=[^"]+")/g, server_url + "$1"); - content = content.replaceAll(/"(\/static\/.*\.js\?v=[^"]+")/g, server_url + "$1"); - content = content.replaceAll(/"(\/packagestatic\/.*\.css\?v=[^"]+")/g, server_url + "$1"); - content = content.replaceAll(/"(\/packagestatic\/.*\.js\?v=[^"]+")/g, server_url + "$1"); - fs.writeFileSync(html_path, content) - }); + if ( !disable_html ) { + let html_path = path; + if (path.endsWith(".jpg")) { + html_path = html_path.substring(0, html_path.length - 4) + ".html"; + } else { + html_path = html_path + ".html"; + } + + // Also save the HTML of the page + await scope.page.content().then(content => { + let server_url = session_vars.get_da_server_url(); + content = content.replaceAll(/"(\/static\/.*\.css\?v=[^"]+")/g, server_url + "$1"); + content = content.replaceAll(/"(\/static\/.*\.js\?v=[^"]+")/g, server_url + "$1"); + content = content.replaceAll(/"(\/packagestatic\/.*\.css\?v=[^"]+")/g, server_url + "$1"); + content = content.replaceAll(/"(\/packagestatic\/.*\.js\?v=[^"]+")/g, server_url + "$1"); + fs.writeFileSync(html_path, content) + }); + } }, // Ends scope.take_a_screenshot() @@ -3151,7 +3170,7 @@ module.exports = { await scope.guard_against_missing_tap_element(scope, { elem }); // Submit and see what happens - let winner = await scope.steps.race_sign_in_navigation( scope, { elem }); + let winner = await scope.steps.race_sign_in_navigation( scope, { elem, login_url }); // Add the result to the report and possibly throw errors if ( winner[0] === `success` ) { reports.addToReport( scope, { @@ -3174,7 +3193,7 @@ module.exports = { } }, // Ends scope.steps.sign_in() - race_sign_in_navigation: async function ( scope, { elem }) { + race_sign_in_navigation: async function ( scope, { elem, login_url='unknown' }) { /** Wait for sign in navigation success or failure, or system error. */ // After everything, clean up incomplete promises @@ -3211,20 +3230,24 @@ module.exports = { }); let click_promise = elem.click(); // MUST complete + let nav_promise = scope.nav_race(scope, {}).result; const winner = await Promise.race([ Promise.all([ `success`, click_promise, redirect_promise ]), Promise.all([ `failure`, click_promise, wrong_sign_in_promise ]), Promise.all([ `error`, click_promise, error_promise ]), - ]).catch(function ( error ) { + ]).catch(async ( error ) => { let error_msg = reports.addToReport( scope, { type: `error`, code: `ALK0207`, value: `Unknown error waiting for results during sign in at ${ login_url }.` }); - throw new Error( error ); + throw error; }); // Clean up unresolved promises controller.abort(); + // Wait to finish navigating + // TODO: Test the winner 'error' under this circumstance + await nav_promise; log.debug({ code: `ALK0208`, context: `nav` }, `Sign-in winner:`, winner @@ -3498,14 +3521,15 @@ module.exports = { let scenario = scope.report.get( scope.scenario_id ); let report = reports.getPrintableScenario( scenario ); let all_are_included = true; + let missing = []; for ( let one_expectation of expected ) { if ( !report.includes( one_expectation )) { all_are_included = false; - expect( report ).to.contain( one_expectation ); + missing.push( one_expectation ); } } - return all_are_included; + return { all_included: all_are_included, missing }; }, // Ends scope.reportIncludesAllExpected() reportDoesNotInclude: async function ( scope, { not_expected=[] }) { diff --git a/lib/steps.js b/lib/steps.js index f3b5bc02..ef39ee3e 100644 --- a/lib/steps.js +++ b/lib/steps.js @@ -114,8 +114,8 @@ BeforeAll(async function() { reports.create( scope ); }); -Before(async (scenario) => { - +Before( beforeScenario ); +async function beforeScenario( scenario ) { // Start the running "progress bar" for the Scenario log.stdout({}, `\nScenario: ${ scenario.pickle.name }: `); @@ -171,7 +171,7 @@ Before(async (scenario) => { // Reset default timeout scope.timeout = default_timeout; -}); +} // Add a check for an error page before each step? After each step? @@ -291,7 +291,6 @@ Given(/I go to "([^"]+)"/i, {timeout: -1}, async ( url ) => { Given( /I (?:sign|log) ?(?:in)?(?:on)?(?:to the server)? with(?: the email)? "([^"]+)",?(?: and)?(?: the password)? "([^"]+)"(?: SECRETs)?(?:,?(?: and)?(?: the API key)? "([^"]+)")?/i, - { timeout: -1 }, async ( email, password, api_key ) => { /** Uses the names of environment variables (most often GitHub SECRETs) to * log into an account on the user's server. Must be secure. @@ -1333,8 +1332,8 @@ AfterStep(async function({ result }) { reports.outdent(); }); -After(async function(scenario) { - +After(afterScenario); +async function afterScenario(scenario) { // Log errors if ( scenario.result.message ) { log.debug({ code: `ALK0091`, context: `scenario`, }, @@ -1379,15 +1378,24 @@ After(async function(scenario) { reports.addToReport(scope, { type: `row info`, code: `ALK0095`, - value: `For security, ALKiln will avoid creating a picture of the page for this error. It's possible a secret is being used on this screen.` + // Discuss: Might people use secret variables to prevent any + // information about a screen from getting out? For example, they may + // have proprietary info in the HTML itself. + value: `For security, ALKiln will avoid creating a picture of the page for this error. It's possible a secret is being used on this screen. ALKiln will save the HTML as a file. The HTML omits the values in the fields.` }); - } else { + } + // Could have an error if page can't load or something similar + try { // Save/download a picture of the screen that's showing during the unexpected status // Save one copy in the outer-most artifact folder let scenario_filename = await scope.getSafeScenarioFilename( scope, { prefix: `error_on` }); let path_outer = `${ scope.paths.artifacts }/${ scenario_filename }.jpg`; - await scope.take_a_screenshot( scope, { path: path_outer }); + await scope.take_a_screenshot( scope, { + path: path_outer, + disable_pic: scope.disable_error_screenshot, + disable_html: false, + }); // Save another copy in the artifact's Scenario folder let screenshot_name = `error_on`; @@ -1395,9 +1403,25 @@ After(async function(scenario) { let short_id = `${ id }`.substring(0, 20); screenshot_name += `-${ short_id }`; let path_scenario = `${ scope.paths.scenario }/${ screenshot_name }.jpg`; - await scope.take_a_screenshot( scope, { path: path_scenario }); + await scope.take_a_screenshot( scope, { + path: path_scenario, + disable_pic: scope.disable_error_screenshot, + disable_html: false, + }); + + } catch ( page_error ) { + // Fail silently. Our inability to take a pic shouldn't cause confusion + // about why a test failed + if ( page_error.message.lower().includes(`execution context`) ) { + reports.addToReport(scope, { + type: `row warning`, + code: `ALK0281`, + value: `ALKiln is unable to get any record of this page whatsoever, even the page HTML. Your server may be busy.` + }); + } + log.debug({ code: `ALK0282`, level: `note`, }, page_error ); + } - } // ends if scope.disable_error_screenshot } // ends if scope.page exists // This has to come after the security message or security message doesn't print. Not sure why. @@ -1417,13 +1441,24 @@ After(async function(scenario) { // ---------------- Check internal test results ---------------- if ( scope.expected_in_report && scope.expected_in_report.length > 0) { - let report_includes_all_expected_strings = await scope.reportIncludesAllExpected( + let data = await scope.reportIncludesAllExpected( scope, { expected: scope.expected_in_report } ); - if ( !report_includes_all_expected_strings ) { changeable_test_status = `FAILED`; } + let report_includes_all_expected_strings = data.all_included; + if ( !report_includes_all_expected_strings ) { + changeable_test_status = `FAILED`; + let msg = `These strings are missing from the report: + ${ JSON.stringify(data.missing, null, 2) } + Instead the report had this text: + ${ report }`; + // These failure messages won't match exactly, which is a shame, but at + // least our devs will have more data _somewhere_ about their failure. + expect( report, msg ).to.contain( data.missing[0] ); + } // Reset report values no matter what so they don't mess up future scenarios scope.expected_in_report = null; + } if (scope.expected_not_in_report && scope.expected_not_in_report.length > 0) { @@ -1605,8 +1640,7 @@ After(async function(scenario) { log.debug({ code: `ALK0100`, context: `scenario`, }, `Scenario After() message:`, scenario.result.message ); - -}); +} // Ends afterScenario() AfterAll(async function() { // Stop collecting server response statuses diff --git a/package.json b/package.json index 5067bd23..43656cc2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@suffolklitlab/alkiln", - "version": "5.16.1", + "version": "5.16.1-always-html-6", "description": "Integrated automated end-to-end testing with docassemble, puppeteer, and cucumber.", "main": "lib/index.js", "scripts": {