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.

Sign up for GitHub

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

Jump to bottom

Fix linter script to run all tags #3400

Merged
merged 2 commits into from
Jul 11, 2018
Merged

Fix linter script to run all tags #3400

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
720b946
Fix linter script to run all tags
sarangan12 Jul 11, 2018
bd1cfb4
PR Comments response
sarangan12 Jul 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions test/linter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,16 @@ async function getTagsFromConfig(config) {
function execLinterCommand(args) {
var cmd = `npx autorest@2.0.4152 --validation --azure-validator --message-format=json ${args}`.trim();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should just use autorest as a library and call the linter through it

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use autorest as a library in the tooling service you can have a look there how to do it, that way also we wont have the autorest version hardcoded in the file

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I will look into it and take it up as a seperate issue.

console.log(`Executing: ${cmd}`);
var errorsFound = false;
try {
let result = execSync(cmd, { encoding: 'utf8', maxBuffer: 1024 * 1024 * 64 });
console.error(result);
} catch (err) {
throw new Error('Linter validation contains error(s)');
errorsFound = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sarangan12 I know we talked a bit offline, but could you provide details on the PR on the bug you're trying to fix? - like what issue you found and how it's not working with the current code.

@dsgouda I believe we worked on adding looping through tags a while back and can't remember why we decided to exit early, do you?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created an issue and provided a detailed explanation and also linked the issue with this PR

console.error('Linter validation contains error(s)');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

print the error also ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Linter itself will print the error. So, printing it again is not required.

}

return errorsFound;
}

describe('AutoRest Linter validation:', function () {
Expand All @@ -102,6 +105,9 @@ describe('AutoRest Linter validation:', function () {

// find all tags in the config file
const tagsToProcess = await getTagsFromConfig(config);

let errorsFound = false;

// if no tags found to process, run with the defaults
if (tagsToProcess === null || tagsToProcess.length === 0) {
// no tags found
Expand All @@ -116,15 +122,21 @@ describe('AutoRest Linter validation:', function () {
return prFile.startsWith(configDir) && prFile.indexOf('examples') === -1 && prFile.endsWith('.json');
}).forEach(prFileInConfigFile => {
console.warn(`WARNING: Configuration file not found for file: ${prFileInConfigFile}, running validation rules against it in individual context.`);
execLinterCommand(`--input-file=${prFileInConfigFile}`);
errorsFound = execLinterCommand(`--input-file=${prFileInConfigFile}`) && errorsFound;
});
}
else {
// if tags found, run linter against every single tag
tagsToProcess.forEach((tagToProcess) => {
execLinterCommand(`${config} --tag=${tagToProcess}`);
errorsFound = execLinterCommand(`${config} --tag=${tagToProcess}`) && errorsFound;
}, this);
}

if(errorsFound == true) {
throw new Error('Linter validation contains error(s)');
}


});
}
}
Expand Down