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.

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

test: don't use exec to test preloaded modules #696

Merged
merged 2 commits into from
Mar 19, 2018

Conversation

kjin
Copy link
Contributor

@kjin kjin commented Mar 19, 2018

It seems like using cp.execSync is a significant source of test flakiness, as the executed command can take more than 4 seconds. The reason for doing this in the past was possibly because it was undesirable to check a global property when all tests ran in a single process. We now run each test in a single process (though if we run tests together again in the future, we will need to ensure that other tests won't interfere with this one).

Also, I added a TestLogger class to easily check logger output. This is used in #686 as well.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 19, 2018
@codecov
Copy link

codecov bot commented Mar 19, 2018

Codecov Report

Merging #696 into master will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #696      +/-   ##
==========================================
+ Coverage   90.08%   90.15%   +0.07%     
==========================================
  Files          29       29              
  Lines        1422     1422              
  Branches      287      287              
==========================================
+ Hits         1281     1282       +1     
  Misses         57       57              
+ Partials       84       83       -1
Impacted Files Coverage Δ
src/trace-writer.ts 88.52% <0%> (+0.81%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5338a93...dfa3c3e. Read the comment docs.

private innerLogger = logger({level: logger.LEVELS[PASS_THROUGH_LOG_LEVEL]});

private makeLoggerFn(logLevel: keyof Logger): LoggerFunction {
// TODO(kjin): When we drop support for Node 4, use spread args.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

private innerLogger = logger({level: logger.LEVELS[PASS_THROUGH_LOG_LEVEL]});

private makeLoggerFn(logLevel: keyof Logger): LoggerFunction {
// TODO(kjin): When we drop support for Node 4, use spread args.

This comment was marked as spam.

@kjin kjin merged commit 770ab08 into googleapis:master Mar 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants