Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Refactored gas tests #289

Merged
merged 53 commits into from Feb 13, 2019
Merged

Refactored gas tests #289

merged 53 commits into from Feb 13, 2019

Conversation

ccowell
Copy link
Contributor

@ccowell ccowell commented Jan 28, 2019

Added missing documentation to bootstrap module
Refactored and grouped gas related tests
Fixed reference errors to old helper files
Fixed a couple of module export definitions

@ccowell ccowell changed the title Refactored gas test Refactored gas tests Jan 28, 2019
test/call.js Outdated Show resolved Hide resolved
@benjamincburns
Copy link
Contributor

So many of these tests are validating that an option was properly handled/observed. Given that this is mostly following a pattern of "set option to nondefault value then validate that the option was set", I wonder if this couldn't/shouldn't be handled by a test helper. That way we could have a single "it observes gas options" test which just runs through a list of options

Copy link
Contributor

@benjamincburns benjamincburns left a comment

Choose a reason for hiding this comment

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

From a high level this looks like a step in the right direction, but I'm a bit confused by some things (see inline comments).

As mentioned in Slack, please don't delete this PR, just submit changes to the same branch so we can preserve the comment history.

test/gas/gas.js Outdated Show resolved Hide resolved
Added toHex to `helpers/utils`
Ignoring .nyc_output
Update references to toBytesHexString
test/call/undefined.js Outdated Show resolved Hide resolved
test/gas/gasLimit.js Outdated Show resolved Hide resolved
test/helpers/contract/compileAndDeploy.js Outdated Show resolved Hide resolved
@ccowell
Copy link
Contributor Author

ccowell commented Feb 12, 2019

So many of these tests are validating that an option was properly handled/observed. Given that this is mostly following a pattern of "set option to nondefault value then validate that the option was set", I wonder if this couldn't/shouldn't be handled by a test helper. That way we could have a single "it observes gas options" test which just runs through a list of options

Extracting the options to a helper file would be ideal and a great start to increasing coverage stats. We can target this during the next refactoring pass through.

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