Fail a11y and internal tests correctly

SuffolkLITLab · Aug 13, 2023 · dee359c · dee359c
1 parent 1137bce
commit dee359c
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 42 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -41,7 +41,12 @@ Format:
 ### Internal
 - 
 -->
-<!-- ## [Unreleased] -->
+## [Unreleased]
+### Fixed
+- Fixed and improved error for accessibility failures. See [#744](https://github.com/SuffolkLITLab/ALKiln/issues/744).
+
+### Internal
+- Changed the format of the aXe file name to shorten it by avoiding repeating the scenario name.
 
 ## [5.1.0] - 2023-08-12
 ### Added

diff --git a/docassemble/ALKilnTests/data/sources/observation_steps.feature b/docassemble/ALKilnTests/data/sources/observation_steps.feature
@@ -135,32 +135,33 @@ Scenario: I take a pic
   Then I take a pic
   Then I take a pic named "some-pic"
 
-# Restore when combobox is accessible, or as a failing test
-# once we get a11y failures working
-# TODO: Create failing a11y test, maybe using custom html
-#@slow @o12 @accessibility @a11y
-#Scenario: I check the pages for accessibility
-#  Given I start the interview at "all_tests"
-#  And I check all pages for accessibility issues
-#  And I tap to continue
-#  Then I get to "screen features" with this data:
-#    | var | value | trigger |
-#    | double_quote_dict["double_quote_key"]['dq_two'] | true |  |
-#    | checkboxes_other['checkbox_other_opt_1'] | true |  |
-#    | combobox_input | Custom combobox option |  |
-#    | dropdown_test | dropdown_opt_2 | |
-#    | radio_yesno | False | false |
-#    | radio_other | radio_other_opt_3 | |
-#    | object_checkboxes_test["obj_chkbx_opt_1"] | True | |
-#    | object_dropdown | obj_opt_2 | |
-#    | object_select_test | obj_chkbx_opt_2 | |
-#    | single_quote_dict['single_quote_key']['sq_two'] | true |  |
-#    | text_input | Regular text input field value | |
-#    | textarea | Multiline text\narea value | |
-#    | date_input | today | |
-#    | button_continue | True |  |
-#    | buttons_other | button_2 |  |
-#    | buttons_yesnomaybe | True |  |
+# Remove `should be "failed"` when docassemble styles are improved
+# for comboboxes.
+# TODO: Create an actual failing a11y test, maybe using custom html
+@slow @o12 @accessibility @a11y
+Scenario: I check the pages for accessibility
+  Given the final Scenario status should be "failed"
+  And I start the interview at "all_tests"
+  And I check all pages for accessibility issues
+  And I tap to continue
+  Then I get to "screen features" with this data:
+    | var | value | trigger |
+    | double_quote_dict["double_quote_key"]['dq_two'] | true |  |
+    | checkboxes_other['checkbox_other_opt_1'] | true |  |
+    | combobox_input | Custom combobox option |  |
+    | dropdown_test | dropdown_opt_2 | |
+    | radio_yesno | False | false |
+    | radio_other | radio_other_opt_3 | |
+    | object_checkboxes_test["obj_chkbx_opt_1"] | True | |
+    | object_dropdown | obj_opt_2 | |
+    | object_select_test | obj_chkbx_opt_2 | |
+    | single_quote_dict['single_quote_key']['sq_two'] | true |  |
+    | text_input | Regular text input field value | |
+    | textarea | Multiline text\narea value | |
+    | date_input | today | |
+    | button_continue | True |  |
+    | buttons_other | button_2 |  |
+    | buttons_yesnomaybe | True |  |
 
 @fast @o13 @signature @screenshot
 Scenario: I take a screenshot of the signature

diff --git a/lib/scope.js b/lib/scope.js
@@ -275,6 +275,15 @@ module.exports = {
     return safe_name;
   },  // Ends scope.getSafeScenarioFilename()
 
+  getA11yPath: async function( scope ) {
+    // Get a unique path for a json file
+    let { id } = await scope.examinePageID( scope, 'none to match' );
+    let short_id = `${ id }`.substring(0, 20);
+    // Need timestamp in case of a loop where the same page gets assessed for a11y multiple times
+    let a11y_path = path.join( scope.paths.scenario, `aXe_failure-${ short_id }-${ Date.now() }.json` );
+    return a11y_path;
+  },
+
   getJSONFilename: async function( scope ) {
     // Get a unique path for a json file
     let { id } = await scope.examinePageID( scope, 'none to match' );
@@ -701,8 +710,7 @@ module.exports = {
     let axe_path = null;
     const passed = axe_result.violations.length == 0;
     if (!passed) {
-      let axe_filename = await scope.getSafeScenarioFilename( scope, {prefix: `aXe_failure`}) + '.json';
-      axe_path = `${ scope.paths.scenario }/${ axe_filename }`;
+      axe_path = await scope.getA11yPath( scope );
       fs.writeFileSync( axe_path, JSON.stringify( axe_result, null, 2 ));
       let msg = `Found potential accessibility issues on the ${ scope.page_id } screen. Details in ${ axe_path }.`;
       await scope.addToReport( scope, {

diff --git a/lib/steps.js b/lib/steps.js
@@ -1097,18 +1097,23 @@ Given(/the final Scenario status should be "(passed|failed)"/i, async ( expected
 
 After(async function(scenario) {
 
+  let changeable_test_status = scenario.result.status;
   // The accessibility failures don't happen til the end, so the rest of the test can continue
   if ( scope.check_all_for_a11y && !scope.passed_all_for_a11y) {
     // The fact that the scenario failed has already been noted earlier in the report
-    scenario.result.status = `FAILED`;
+    changeable_test_status = `FAILED`;
+    scope.addToReport(scope, {
+      type: `error`,
+      value: `Accessibility standards weren't met. See information above.`
+    });
   }
 
   // Record status before internal report tests can interfere with values
-  await scope.setReportScenarioStatus( scope, { status: scenario.result.status });
+  await scope.setReportScenarioStatus( scope, { status: changeable_test_status });
 
-  if (scenario.result.status !== `PASSED`) {
+  if (changeable_test_status !== `PASSED`) {
     if ( session_vars.get_debug() ) {
-      console.log( `Non-passed status occurred while running test:`, scenario.result.status );
+      console.log( `Non-passed status occurred while running test:`, changeable_test_status );
     }
 
     if ( scope.page ) {
@@ -1138,21 +1143,19 @@ After(async function(scenario) {
     }  // ends if scope.page exists
 
     // This has to come after the security message or security message doesn't print. Not sure why.
-    if (scenario.result.status === `FAILED`) {
+    if (changeable_test_status === `FAILED`) {
       await scope.addToReport(scope, { type: `outcome`, value: `**-- Scenario Failed --**` });
     }
 
   }  // ends if not passed
 
-  let original_scenario_status = scenario.result.status;
-
   // Internal tests of this framework
   if ( scope.expected_status ) {
     let expected = scope.expected_status;
     scope.expected_status = null;  // reset for next Scenario
-    expect( scenario.result.status ).to.equal( expected );
+    expect( changeable_test_status ).to.equal( expected );
     // If the status was as expected, pass this criteria
-    scenario.result.status = `PASSED`;
+    changeable_test_status = `PASSED`;
   }
 
   if ( scope.expected_in_report && scope.expected_in_report.length > 0) {
@@ -1161,8 +1164,8 @@ After(async function(scenario) {
     scope.expected_in_report = null;
     let all_were_included = await scope.reportIncludesAllExpected( scope, { expected });
 
-    if ( all_were_included ) { scenario.result.status = `PASSED`; }
-    else { scenario.result.status = `FAILED`; }
+    if ( all_were_included ) { changeable_test_status = `PASSED`; }
+    else { changeable_test_status = `FAILED`; }
   }
 
   if (scope.expected_not_in_report && scope.expected_not_in_report.length > 0) {
@@ -1172,9 +1175,9 @@ After(async function(scenario) {
     let all_were_not_there = await scope.reportDoesNotInclude( scope, { not_expected });
 
     if ( all_were_not_there ) {
-      scenario.result.status = `PASSED`;
+      changeable_test_status = `PASSED`;
     } else { 
-      scenario.result.status = `FAILED`;
+      changeable_test_status = `FAILED`;
     }
 
   }
@@ -1212,6 +1215,19 @@ After(async function(scenario) {
   let report = await scope.getPrintableScenario( scope, { scenario: scenario_report_obj });
   // Save the report as in the Scenario folder
   fs.writeFileSync( scope.paths.scenario_report, report );
+
+  if ( changeable_test_status === `FAILED` ) {
+    // If the result object status wasn't failure and we still want
+    // the tests to fail, as with a11y tests, we can't mess with the
+    // cucumber result.status object because it will cause unwanted
+    // errors. We have to throw an error here instead.
+    if ( scenario.result.status !== `FAILED` ) {
+      throw new Error(`The test failed. See the report for more information.`);
+    }
+  } else {
+    // Other types of errors should be ok
+    scenario.result.status = changeable_test_status;
+  }
 });
 
 AfterAll(async function() {