Results being overwritten when same url but different actions

[stbth01]

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.

"http://localhost:4200/landing/topics/info",
        {
            "url": "http://localhost:4200/landing/topics/info",

            "actions": [
                "wait for url to be http://localhost:4200/landing/topics/info",
                "click element label[data-cy=compare-measure-view-link]",
                "click element label[data-cy=single-measure-view-link]",
                "click element .topic-area-option:first-child",
                "click element .topic-option:first-child",
                "wait for url to be http://localhost:4200/landing/topics/single",
                "wait for element app-topics-map to be added",
                "screen capture pa11y/topics-map.png",
                "click element .year-dropdown svg.icon",
                "click element .path-WA"
            ]
        },
        {
            "url": "http://localhost:4200/landing/topics/info",
            "actions": [
                "wait for url to be http://localhost:4200/landing/topics/info",
                "click element .topic-area-option:first-child",
                "click element .topic-option:first-child",
                "wait for url to be http://localhost:4200/landing/topics/single",
                "click element app-toggle-visualization #vis-label1",
                "wait for element app-table to be added",
                "screen capture pa11y/topic-table.png",
                "click element .issue-briefs-title"
            ]
        }

This gives the following result if the third test failed even if the first two had errors

 "http://localhost:4200/landing/topics/info": []

[kkoskelin]

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.

[aarongoldenthal]

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:

• 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.

I like change the behavior approach because then results are consistently reported. There's already precedence for output with different URLs (see #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.

[aarongoldenthal]

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.

[joeyciechanowicz]

• 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.

[aarongoldenthal]

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).

[aarongoldenthal]

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.