Open stbth01 opened 4 years ago
I have a large set of tests, some of which test the same SPA url but in different states. We have adopted a practice of appending ?info=testing-search-form
etc query string to distinguish one from the other. I know that's fixing the symptom only, but it has been helpful for us.
This issue can be especially confusing because different reporters can report different results based on the which events are implemented. The core issue is that the results for a URL in the aggregated results
object are overwritten if the same URL is analyzed again (here or here). That results
object is then passed to the afterAll
reporter event, so cases like the JSON reporter show only one result.
But, the reporters that handle the begin
/results
/error
events as URLs are processed do show (or at least process) the results for each (at least for those events). So, looking at a case like the CLI reporter, the summary of each URL is handled in the results
/error
events and shows the summary for all URLs, then the detailed list is output in afterAll
, which will only have the results for the last URL. So running something like:
{
"defaults": {
"includeWarnings": true
},
"urls": [
"https://pa11y.org/",
{
"url": "https://pa11y.org/",
"timeout": 5
}
]
}
results in:
Running Pa11y on 2 URLs:
> https://pa11y.org/ - 2 errors
> https://pa11y.org/ - Failed to run
Errors in https://pa11y.org/:
• Error: Pa11y timed out (5ms)
✘ 0/2 URLs passed
I've used the same technique that @kkoskelin mentioned above of appending querystring parameters to differentiate, and usually I want that because it lets me identify what I did in actions
that I'm checking for. It can definitely be confusing though, especially for those unfamiliar with the behavior and which reporter is being used, so I propose one of the following:
I like change the behavior approach because then results are consistently reported. There's already precedence for output with different URLs (see https://github.com/pa11y/pa11y-ci/issues/172).
As an indication of the very subtle differences you can see now, running the previous example removing the trailing slash on the URL:
{
"defaults": {
"includeWarnings": true
},
"urls": [
"https://pa11y.org",
{
"url": "https://pa11y.org",
"timeout": 5
}
]
}
Does results in the correct output (since the first is successful, so the reported URL is document.location.href
, which the browser normalizes):
Running Pa11y on 2 URLs:
> https://pa11y.org/ - 2 errors
> https://pa11y.org - Failed to run
Errors in https://pa11y.org/:
• Img element is marked so that it is ignored by Assistive Technology.
(html > body > div > header > a > img)
• The heading structure is not logically nested. This h3 element should be an h2 to be properly nested.
(#pa11y-1)
<h3 id="pa11y-1"><a href="https://github.com/pa1...</h3>
Errors in https://pa11y.org:
• Error: Pa11y timed out (5ms)
✘ 0/2 URLs passed
@joeyciechanowicz @josebolos Thoughts on this?
Edited to correct the config to include warnings, which are what's represented in the output.
One follow-up thought - in some cases the querystring is modified by the app through actions
, so any augmentation is lost by the end, and since the results are save based on document.location.href
they're overwritten.
- Change the behavior approach: Update the results aggregation to look for the URL in the existing results and add the new results under a distinct URL appending some incrementing suffix. The README should probably also be updated to detail what the output means.
- Clarify the behavior approach: Update the README to specifically identify the behavior and the querystring workaround. With that, it would be helpful to update the results aggregation to look for the URL in the existing results and output a warning when results are overwritten.
The query string workaround seems more brittle, due to it breaking when the browser or actions update it
My vote would be for the first option, to detect duplicates. However with a slight modification, that it passes an incremented identifier as a description
field. We can then allow the description
to be set in the config on a per URL basis. Duplicate tuples of url + description
would have an incremented ID appended.
This approach would fix the problem, bit also allow users to uniquely identify results as they see best.
Adding some kind of description
field in config was something I was thinking about as well after playing around with it. The duplicate detection and adding the counter is actually pretty straightforward (a working example here, minus tests and documentation).
I wanted to put this out for any comments on formatting or approach. I have an update that accepts an optional description
field in the url
config, if present it's combined with the url
as the key in the results, and an incrementing ID added for multiples.
As an illustration, this config:
{
"defaults": {
"reporters": [["json", { "fileName": "./results.json" }]]
},
"urls": [
"https://pa11y.org",
{
"url": "https://pa11y.org",
"description": "test something"
},
{
"url": "https://pa11y.org",
"description": "test something else"
},
{
"url": "https://pa11y.org",
"description": "test something"
}
]
}
results in:
{
"total": 4,
"passes": 4,
"errors": 0,
"results": {
"https://pa11y.org/": [],
"https://pa11y.org/ - test something": [],
"https://pa11y.org/ - test something else": [],
"https://pa11y.org/ - test something (2)": []
}
}
This processing is done when saving results, so only effects the object passed to the afterAll
reporter event. The other applicable reporter events do get the url-specific config, so they'd have access to both the url
and description
values (the object passed to results
comes from pa11y
directly with the pageUrl
property, and is the same as pa11y reporters, so seemed better to leave it alone).
I assume the documentation would list description
as a pa11y-ci specific configuration property since pa11y isn't using it unless there's a desire to make changes to pa11y as well.
I have a SPA so I would like to test different actions that don't necessarily change the URL. However when I see the results I only see the errors from the most recent test. I have a piece of my config below as an example.
This gives the following result if the third test failed even if the first two had errors