Documentation and code mismatch when using sitemap and config

[mkotamies]

pa11y-ci Github page has following note: Providing a sitemap will cause the urls property in your JSON config to be ignored. Looks like the statement is incorrect. Providing sitemap as a parameter and having config with uurls, actually includes the urls in the test.

Here is example repo with instructions to replicate the problem.

At the moment, there isn't a test case with config and sitemap. Add the following test to cli-sitemap.test.js and the test will fail.

describe('pa11y-ci (with a sitemap being sitemapindex) and config. should ignore config urls', () => {

    before(() => {
        return global.cliCall([
            '--sitemap',
            'http://localhost:8090/sitemapindex.xml',
            '--config',
            'extension-json'
        ]);
    });

    it('loads the expected urls from multiple sitemaps', () => {
        assert.notInclude(global.lastResult.output, 'http://localhost:8090/config-extension-json');
        assert.include(global.lastResult.output, 'http://localhost:8090/passing-1');
        assert.include(global.lastResult.output, 'http://localhost:8090/failing-1');
        assert.include(global.lastResult.output, 'http://localhost:8090/excluded');
        assert.include(global.lastResult.output, 'http://localhost:8090/passing-2');
    });

});

I'm not sure if the documentation or code is correct. My assumption is that the documentation is correct

[mkotamies]

Here's a potential solution with a test case

[aarongoldenthal]

In looks like the README was changed in a documentation cleanup between v3.0.1 and v3.1.0 here:

- This takes the text content of each `<loc>` in the XML and runs Pa11y against that URL.
- This can also be combined with a config file, but URLs in the sitemap will override any
- found in your JSON config.
+ Pa11y will be run against the text content of each `<loc/>` in the sitemap's XML.
+
+ > NOTE
+ > Providing a sitemap will cause the `urls` property in your JSON config to be ignored.

The code actually doesn't behave either way - it analyzes both (using any other specified config that's available for that case). If specified from the CLI as well it will add a third.

I would advocate leaving the behavior as-is and cleaning up the documentation, but if a change is made then I would think the opposite is more beneficial - the config urls should override the sitemap. The config allows a much broader set of options to be specified, and would then allow overriding the config only for specific URLs in a sitemap (e.g. increasing the timeout or adding a wait for one URL).

[mkotamies]

That's a good finding from the history!

As I mentioned in the first message, I'm not sure if the docs or code is correct. Cleaning up the documentation makes sense and it'll remove the confusion between the two

[danyalaytekin]

Hi both and thank you, including for the extremely helpful repo and test case @mkotamies, and your additional understanding and information @aarongoldenthal.

---

@aarongoldenthal

The code actually doesn't behave either way - it analyzes both (using any other specified config that's available for that case). I would advocate leaving the behavior as-is and cleaning up the documentation, but if a change is made then I would think the opposite is more beneficial - the config urls should override the sitemap.

I agree that while the current behaviour of loading exact duplicates isn't ideal, it does solve the problem of 'overrides' that you've described when the accompanying settings vary.

---

I've moved this into 4.x but it may well ship with 4.0.0 if it's completed in time.