Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

[sol-cov] add ability to ignore covering specific code blocks #766

Merged
merged 4 commits into from
Jun 27, 2018

Conversation

ewingrj
Copy link
Contributor

@ewingrj ewingrj commented Jun 22, 2018

Description

Adds functionality similar to istanbul's ignore to exclude a block of code from the coverage report

Testing instructions

add /* solcov ignore next */ before any block of code in the solidity source to exclude that block from the coverage report

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Change requires a change to the documentation.
  • Added tests to cover my changes.
  • Added new entries to the relevant CHANGELOG.jsons.
  • Labeled this PR with the 'WIP' label if it is a work in progress.
  • Labeled this PR with the labels corresponding to the changed package.

@@ -23,8 +23,11 @@ export class ASTVisitor {
private _modifiersStatementIds: number[] = [];
private _statementMap: StatementMap = {};
private _locationByOffset: LocationByOffset;
constructor(locationByOffset: LocationByOffset) {
private _ignoreRangesBeginingAt: number[];
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: Beginning

@LogvinovLeon
Copy link
Contributor

This looks good. Can we add some tests for it?

@fabioberger
Copy link
Contributor

@perissology I tried using this PR on the contracts package tests and thought I would be able to add /* solcov ignore next */ right above a Contract declaration to ignore the entire contents of a contract, but it didn't work. Does it not work for ignoring entire contracts?

@ewingrj
Copy link
Contributor Author

ewingrj commented Jun 25, 2018

I added support for ignoring contracts, as well as tests

@@ -42,6 +47,11 @@ export class ASTVisitor {
public FunctionDefinition(ast: Parser.FunctionDefinition): void {
this._visitFunctionLikeDefinition(ast);
}
public ContractDefinition(ast: Parser.ContractDefinition): void {
if (this._ignoreExpression(ast)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename _ignoreExpression to _shouldIgnoreExpression

parser.visit(ast, visitor);
coverageEntriesBySourceHash[sourceHash] = visitor.getCollectedCoverageEntries();
}
const coverageEntriesDescription = coverageEntriesBySourceHash[sourceHash];
return coverageEntriesDescription;
};

const IGNORE_RE = /\/\*\s*solcov\s+ignore\s+next\s*\*\/\s*/gm;
Copy link
Contributor

Choose a reason for hiding this comment

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

Put constants at top of the file.

const setFunction = `function set(uint x) public {
/* solcov ignore next */
storedData = x;
}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

If indentation doesn't matter here, let's indent this further so that it's easier to read.

Copy link
Contributor

Choose a reason for hiding this comment

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

It does matter here

@@ -121,5 +121,35 @@ describe('Collect coverage entries', () => {
const branchTypes = _.map(branchDescriptions, branchDescription => branchDescription.type);
expect(branchTypes).to.be.deep.equal(['if', 'if', 'if', 'if', 'binary-expr', 'if']);
});

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove excess new line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

prettier adds that line

Copy link
Contributor

@fabioberger fabioberger left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes!

@fabioberger fabioberger merged commit 57d5fbf into 0xProject:v2-prototype Jun 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants