Skip to content

test: clean up repl-reset-event file#9931

Closed
courtnek wants to merge 1 commit into
nodejs:masterfrom
courtnek:codeNLearn
Closed

test: clean up repl-reset-event file#9931
courtnek wants to merge 1 commit into
nodejs:masterfrom
courtnek:codeNLearn

Conversation

@courtnek

@courtnek courtnek commented Dec 1, 2016

Copy link
Copy Markdown
Contributor
Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)
Description of change
  • Change vars to let/const
  • Add mustCall
  • equal -> strictEqual
  • remove timeout

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Dec 1, 2016
@imyller imyller added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Dec 1, 2016
@mscdex mscdex added the repl Issues and PRs related to the REPL subsystem. label Dec 1, 2016
Comment thread test/parallel/test-repl-reset-event.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you line this up with the previous line.

@Trott

Trott commented Dec 10, 2016

Copy link
Copy Markdown
Member

Ping @courtnek: Thanks for the work on this! Can you make the change requested by @cjihrig and push it to this branch?

If you want, you can copy/paste this into the file:

'use strict';
const common = require('../common');
common.globalCheck = false;

const assert = require('assert');
const repl = require('repl');

// Create a dummy stream that does nothing
const dummy = new common.ArrayStream();

function testReset(cb) {
  const r = repl.start({
    input: dummy,
    output: dummy,
    useGlobal: false
  });
  r.context.foo = 42;
  r.on('reset', common.mustCall(function(context) {
    assert(!!context, 'REPL did not emit a context with reset event');
    assert.strictEqual(context, r.context, 'REPL emitted incorrect context');
    assert.strictEqual(
      context.foo,
      undefined,
      'REPL emitted the previous context, and is not using global as context'
    );
    context.foo = 42;
    cb();
  }));
  r.resetContext();
}

function testResetGlobal() {
  const r = repl.start({
    input: dummy,
    output: dummy,
    useGlobal: true
  });
  r.context.foo = 42;
  r.on('reset', common.mustCall(function(context) {
    assert.strictEqual(
      context.foo,
      42,
      '"foo" property is missing from REPL using global as context'
    );
  }));
  r.resetContext();
}

testReset(common.mustCall(testResetGlobal));

@courtnek

Copy link
Copy Markdown
Contributor Author

Hey @cjihrig @Trott,

Sorry for the delay!

Yeah @Trott I like breaking up the function args over multiple lines over splitting the string. I amended my commit with the new style.

Let me know if there is anything else I can do.

* Change vars to let/const
* Add mustCall
* equal -> strictEqual
* remove timeout
@Trott

Trott commented Dec 21, 2016

Copy link
Copy Markdown
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/5519/

@cjihrig Looks like your comment has been addressed. Looks good to you now?

@Trott

Trott commented Dec 22, 2016

Copy link
Copy Markdown
Member

CI is ✅

@jasnell

jasnell commented Dec 24, 2016

Copy link
Copy Markdown
Member

ping @cjihrig

jasnell pushed a commit that referenced this pull request Dec 24, 2016
* Change vars to let/const
* Add mustCall
* equal -> strictEqual
* remove timeout

PR-URL: #9931
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@jasnell

jasnell commented Dec 24, 2016

Copy link
Copy Markdown
Member

Landed in e81e031

@jasnell jasnell closed this Dec 24, 2016
@Trott

Trott commented Dec 24, 2016

Copy link
Copy Markdown
Member

Thanks for the contribution, @courtnek! 🎉

targos pushed a commit that referenced this pull request Dec 26, 2016
* Change vars to let/const
* Add mustCall
* equal -> strictEqual
* remove timeout

PR-URL: #9931
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
targos pushed a commit that referenced this pull request Dec 26, 2016
* Change vars to let/const
* Add mustCall
* equal -> strictEqual
* remove timeout

PR-URL: #9931
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This was referenced Dec 27, 2016
evanlucas pushed a commit that referenced this pull request Jan 3, 2017
* Change vars to let/const
* Add mustCall
* equal -> strictEqual
* remove timeout

PR-URL: #9931
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
evanlucas pushed a commit that referenced this pull request Jan 4, 2017
* Change vars to let/const
* Add mustCall
* equal -> strictEqual
* remove timeout

PR-URL: #9931
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. repl Issues and PRs related to the REPL subsystem. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants