SuffolkLITLab / ALKiln

Integrated automated end-to-end testing with docassemble, puppeteer, and cucumber.
https://assemblyline.suffolklitlab.org/docs/alkiln/intro
MIT License
14 stars 4 forks source link

Accessibility failures causes a weird output and an error after all the (usually) final cucumber message #744

Closed plocket closed 7 months ago

plocket commented 1 year ago

When we gather a11y failures for a Scenario, we wait till the After() to fail the test. The way we fail the test is by setting the scenario.result.status to FAILED. When we do that, cucumber expects to find a failure message (.message) in its own internal steps objects (discovered by exploring their code in test_case_attempt_formatter), so instead of failing in a way we recognize, we get something like the following:

1) Scenario: My test # test.feature:line
   ✖ Before # steps.js:line
       undefined
   ✔ Given I start the interview at "all_tests" # steps.js:line
   ✔ And I check all pages for accessibility issues # steps.js:line
   ✔ And another step # steps.js:line
   ✔ And another step # steps.js:line
   ✔ After # steps.js:line

1 scenario (1 failed)
4 steps (4 passed)
0m17.605s (executing steps: 0m17.496s)
/Users/repo/node_modules/indent-string/index.js:11
    throw new TypeError(
          ^

TypeError: Expected `input` to be a `string`, got `undefined`
    at module.exports (/Users/repo/node_modules/indent-string/index.js:11:9)
    at formatStep (/Users/repo/node_modules/@cucumber/cucumber/lib/formatter/helpers/test_case_attempt_formatter.js:87:48)
    at /Users/repo/node_modules/@cucumber/cucumber/lib/formatter/helpers/test_case_attempt_formatter.js:101:17
    at Array.forEach (<anonymous>)
    at formatTestCaseAttempt (/Users/repo/node_modules/@cucumber/cucumber/lib/formatter/helpers/test_case_attempt_formatter.js:100:22)
    at formatIssue (/Users/repo/node_modules/@cucumber/cucumber/lib/formatter/helpers/issue_helpers.js:25:94)
    at /Users/repo/node_modules/@cucumber/cucumber/lib/formatter/summary_formatter.js:55:48
    at Array.forEach (<anonymous>)
    at SummaryFormatter.logIssues (/Users/repo/node_modules/@cucumber/cucumber/lib/formatter/summary_formatter.js:54:16)
    at SummaryFormatter.logSummary (/Users/repo/node_modules/@cucumber/cucumber/lib/formatter/summary_formatter.js:41:18)

Node.js v18.17.0

Adding a scenario.status.message does seem to remove that error that appears after the tests complete, though the x still appears in the same place. That says, the undefined gets changed to our message. I'm not sure we should keep manipulating the internals of the tests this way, though, and while our output looks ok the cucumber output is a bit confusing.

[Also, I think tests failing because of accessibility issues may not actually fail the whole test suite, so we should check that.]

A different way to handle this would be to create a new Step to check for the passing of all the accessibility checks, e.g. And all the pages were accessible. This would be more confusing to the user, unfortunately, but I can't think of another way to handle it that fits into the constraints cucumber presents.

Tangentially: This will also be a problem for some of our internal test failures because we're setting their status in a similar way.

plocket commented 1 year ago

Made this branch at this line with some of my experimentation that should solve the problem, though the tests aren't showing the behavior really - I didn't get to add the combobox into the test files yet. Also, we should expect that test to fail. Also, maybe the test won't properly fail because of the weirdness that's going on, so we should watch out for that.

plocket commented 1 year ago

I wonder if this would be useful: https://github.com/cucumber/cucumber-js/issues/1156

Suggestion there is:

After((world, callback) => {
  callback(null, 'skipped');
});

~Edit: Maybe also something in modifying test results issue at the bottom or linked in the new functionality (though I think that new functionality may actually not be capable of this).~ I've been told we shouldn't modify the object handed in to the After() callback, though they're looking into the After() callback's callback (above) to see if that would work.

plocket commented 1 year ago

The failure seems to be on the combobox toggle. [Just fyi if we're hoping to fix that accessibility issue.]

aXe_failure-direct-standard-fiel-I check the pages for accessibility-1691873465488.txt

plocket commented 1 year ago

For now, we'll throw an error at the end of the After() step instead of altering the status manually. That way the tests will fail with our own custom error message in the cucumber output.

BryceStevenWilley commented 11 months ago

For what it's worth: the combo box issue was fixed upstream in docassemble in 1.4.69.

A different way to handle this would be to create a new Step to check for the passing of all the accessibility checks, e.g. And all the pages were accessible. This would be more confusing to the user, unfortunately, but I can't think of another way to handle it that fits into the constraints cucumber presents.

Is this something we want to do? Not a huge fan, but if it makes the error messages a lot cleaner I suppose we could do it?

plocket commented 11 months ago

It's just one way. If we can think of others, I'd prefer those.

plocket commented 11 months ago

I'm not sure I mentioned it, but someone in the cucumber Slack mentioned changing behavior in a way that would prevent us from doing what we currently do.

plocket commented 7 months ago

Closed by #750