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

[sol-cov] Only collect coverage for provided sources #702

Merged
merged 3 commits into from
Jun 21, 2018

Conversation

ewingrj
Copy link
Contributor

@ewingrj ewingrj commented Jun 14, 2018

Motivation and Context

When solidity generates source maps during contract compilation, the
contracts are referred to by an id, which corresponds to an array index.

We may not want to cover all sources that were included in a compilation,
but because we use array indexes (vs. the id that is provided by solidity
compiler) to map the contract to the sourceMap, the provided sourceCodes
array must include the code at the correct index. This can result in
empty slots in the sourceCodes array.

This commit allows the coverage to only be collected for the contracts
with provided sourceCode.

How Has This Been Tested?

Tested with our repos. I was not able to run yarn test as it failed before I made any changes

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.

When solidity generates source maps during contract compilation, the
contracts are referred to by an id, which corresponds to an array index.

We may not want to cover all sources that were included in a compilation,
but because we use array indexes (vs. the id that is provided by solidity
compiler) to map the contract to the sourceMap, the provided sourceCodes
array must include the code at the correct index. This can result in
empty slots in the sourceCodes array.

This commit allows the coverage to only be collected for the contracts
with provided sourceCode.
@ewingrj ewingrj changed the title Only collect coverage for provided sources [sol-cov] Only collect coverage for provided sources Jun 14, 2018
Copy link
Contributor

@LogvinovLeon LogvinovLeon left a comment

Choose a reason for hiding this comment

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

LGTM except for one nit.

@@ -12,6 +12,9 @@ export interface SourceLocation {
}

export function getLocationByOffset(str: string): LocationByOffset {
if (_.isUndefined(str)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we check that above and just not call this function?
I'd prefer that solution because now we're checking for undefined the variable that's guaranteed to be undefined by the type system. Let's either change the types or check before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, I checked before

@LogvinovLeon
Copy link
Contributor

Prettier fails on CI. Please run yarn prettier to fix formatting

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

2 participants