Skip to content

add garbage collection tests, and fix scrubFields #57

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
Merged
merged 2 commits into from
Jan 10, 2017
Merged

add garbage collection tests, and fix scrubFields #57

merged 2 commits into from
Jan 10, 2017

Conversation

@mattjstar
Copy link
Contributor

@mattjstar mattjstar commented Jan 10, 2017 •
edited
Loading

Fixes regression introduced in 0.5.0 #54

  • Adds garbage collection tests using leakage
  • Adds test:leakage script to run leakage tests only (keep in separate process)
  • Moves all other specs to common folder to keep test:browser and test:node the same
  • Changes scrubFields functionality from inline anonymous function to imported function

@coveralls
Copy link

coveralls commented Jan 10, 2017

Coverage Status

Coverage increased (+0.5%) to 77.174% when pulling 68ec1e1247661933aa6ab26e099b36b9203ee15a on bugfix/leakage into 3d5e72f on master.

@coveralls
Copy link

coveralls commented Jan 10, 2017

Coverage Status

Coverage increased (+0.5%) to 77.174% when pulling 963a382 on bugfix/leakage into 3d5e72f on master.

ricardo-quinones
ricardo-quinones approved these changes Jan 10, 2017
View reviewed changes
Copy link
Contributor

@ricardo-quinones ricardo-quinones left a comment

Choose a reason for hiding this comment

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

Dope son 🎊

nason
nason approved these changes Jan 10, 2017
View reviewed changes
Copy link
Contributor

@nason nason left a comment

Choose a reason for hiding this comment

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

Awesome!

package.json Outdated
"test:node": "NODE_ENV=test nyc mocha test/runner test/specs/common/**/*.js",
"test:node:tdd": "npm run test:node -- --cache --watch",
"test:node:ci": "npm run test:node && nyc report --reporter=text-lcov | coveralls",
"test:leakage": "NODE_ENV=test nyc mocha test/runner test/specs/leakage/**/*.js",
Copy link
Contributor

@nason nason Jan 10, 2017

Choose a reason for hiding this comment

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

Can you take "nyc" out of this target? Coverage instrumentation might affect this test, and I don't know if that's important here

@nason
Copy link
Contributor

nason commented Jan 10, 2017

Opened #58 as a follow up

i8ramin
i8ramin approved these changes Jan 10, 2017
View reviewed changes
Copy link
Contributor

@i8ramin i8ramin left a comment

Choose a reason for hiding this comment

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

amaze 🏀 ⚾️ 's

@coveralls
Copy link

coveralls commented Jan 10, 2017

Coverage Status

Coverage increased (+0.5%) to 77.174% when pulling 47cde2a on bugfix/leakage into 3d5e72f on master.

@mattjstar mattjstar merged commit 40086b4 into master Jan 10, 2017
@mattjstar mattjstar deleted the bugfix/leakage branch January 10, 2017 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@i8ramin i8ramin i8ramin approved these changes

@nason nason nason approved these changes

@ricardo-quinones ricardo-quinones ricardo-quinones approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mattjstar @coveralls @nason @i8ramin @ricardo-quinones