Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow connector opt-out for individual tests #250

Closed
molant opened this issue Jun 8, 2017 · 2 comments
Closed

Allow connector opt-out for individual tests #250

molant opened this issue Jun 8, 2017 · 2 comments

Comments

@molant
Copy link
Member

molant commented Jun 8, 2017

Right now we can ignore a rule from being run and/or tested via the ignoredCollectors properties.
Sometimes the differences are more granular and there are just some scenarios when we are testing that do not work on a specific collector and maybe it is just a question of waiting to the next release. Instead of just commenting the test (and thus not test anywhere) we should add the option to opt-out for an specific collector.
An example will be the content-type tests. Currently Chrome doesn't download JavaScript modules (it's behind a flag in v60). Also it doesn't download anything if it doesn't like the type attribute

We should also add a //TODO comment so we remove it later (hopefully once #210 is implemented so things are more automated).

I'm thinking about something similar to:

 {
        name: `Script is served with 'Content-Type' header with the wrong media type (has 'type=text/plain' and 'js' file extension)`,
        reports: [{ message: generateIncorrectMediaTypeMessage('application/javascript', 'text/plain') }],
        serverConfig: {
            '/': generateHTMLPageData(generateHTMLPage(undefined, '<script type="text/plain" src="test.js"></script>')),
            '/test.js': { headers: { 'Content-Type': 'text/plain; charset=utf-8' } }
        },
        ignoredCollectors: ['cdp']
    },

@sarvaje I don't know how you are implementing the Edge Collector (#170) but if we want this to work it will need a different id and not just a config change.

@MicrosoftEdge/sonar-devs thoughts?

@sarvaje
Copy link
Contributor

sarvaje commented Jun 8, 2017

@sarvaje I don't know how you are implementing the Edge Collector (#170) but if we want this to work it will need a different id and not just a config change.

I'm thinking to have cdp and another one... maybe eda or edge and they will share as much code as possible

@molant molant changed the title Allow collector opt-out for individual tests Allow connector opt-out for individual tests Jul 20, 2017
@molant
Copy link
Member Author

molant commented Oct 24, 2017

After thinking a bit more, I don't think we need this. It doesn't happen that often and when we have problems we should group the failing tests and just use the regular ignoredConnectors property of testRule:

const tests = [...];
const testsNotRunningOnConnector = [...];

ruleRunner.testRule(ruleName, tests);
ruleRunner.testRule(ruleName, testsNotRunningOnConnector, { ignoredConnectors: ['connector']});

@molant molant closed this as completed Oct 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants