Skip to content

test: increase test coverage of readline-interface #16062

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
Closed
stropitek wants to merge 1 commit into from
Closed

test: increase test coverage of readline-interface #16062

stropitek wants to merge 1 commit into from

Conversation

@stropitek
Copy link
Contributor

@stropitek stropitek commented Oct 7, 2017 •
edited
Loading

Adds coverage for:

  • question callback
  • history navigation
  • bad historySize option
  • multi-line output
  • history stays bound and most recent elements are preserved
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

@stropitek
test: increase test coverage of readline-interface
6b08ffe 
Adds coverage for:
- question callback
- history navigation
- bad historySize option
- multi-line output
- history is bound and most recent elements are preserved
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 7, 2017
@mscdex mscdex added the readline Issues and PRs related to the built-in readline module. label Oct 7, 2017
@Trott Trott added the code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. label Oct 7, 2017
@addaleax
Copy link
Member

addaleax commented Oct 7, 2017

CI: https://ci.nodejs.org/job/node-test-commit/12842/

BridgeAR
BridgeAR reviewed Oct 9, 2017
View reviewed changes
test/parallel/test-readline-interface.js
}, common.expectsError({
type: RangeError,
code: 'ERR_INVALID_OPT_VALUE'
}));
Copy link
Member

@BridgeAR BridgeAR Oct 9, 2017

Choose a reason for hiding this comment

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

It would be nice to use common.expectsError(throwingFn, errorObj) instead of assert.throws(throwingFn, common.expectsError(errorObj)).

@lance lance self-assigned this Oct 9, 2017
@lance
Copy link
Member

lance commented Oct 9, 2017

Two known-flakey tests failed. Landing this.

vs2017,win2016,0 
not ok 465 inspector/test-stop-profile-after-done # TODO : Fix flaky test
  ---
  duration_ms: 0.448
  severity: flaky
  stack: |-
    [test] Connecting to a child Node process
    [test] Testing /json/list
    [err] Debugger listening on ws://127.0.0.1:62852/d26d5a06-82ab-4e05-bf7e-9e418da4b1b3
    [err] For help see https://nodejs.org/en/docs/inspector
    [err] 
    [out] {}
    [out] 
    [out] {}
    [out] 
    [out] {}
    [out] 
    [out] {}
    [out] 
    [out] {}
    [out] 
    [out] {}
    [out] 
    [out] {}
    [out] 
    [out] {}
    [out] 
    [out] {}
    [out] 
    [out] {}
    [out] 
    [out] {}
    [out] 
    [err] Debugger attached.
    [err] Waiting for the debugger to disconnect...
    [err] 
    { AssertionError [ERR_ASSERTION]: 0 === 3221225477
        at runTests (c:\workspace\node-test-binary-windows\COMPILED_BY\vs2017\RUNNER\win2016\RUN_SUBSET\0\test\inspector\test-stop-profile-after-done.js:26:10)
        at 
        at process._tickCallback (internal/process/next_tick.js:188:7)
      generatedMessage: true,
      name: 'AssertionError [ERR_ASSERTION]',
      code: 'ERR_ASSERTION',
      actual: 0,
      expected: 3221225477,
      operator: '===' }
    1
  ...
vs2017,win2016,1 
not ok 462 inspector/test-bindings # TODO : Fix flaky test
  ---
  duration_ms: 0.222
  severity: crashed
  stack: |-
    oh no!
    exit code: CRASHED

lance pushed a commit that referenced this pull request Oct 9, 2017
@stropitek @lance
test: increase test coverage of readline-interface
f2b9d5e 
Adds coverage for:
- question callback
- history navigation
- bad historySize option
- multi-line output
- history is bound and most recent elements are preserved

PR-URL: #16062
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@lance
Copy link
Member

lance commented Oct 9, 2017

Landed in f2b9d5e

@stropitek thanks a lot for your contribution! It's very appreciated.

@lance lance closed this Oct 9, 2017
@stropitek stropitek deleted the test-readline branch October 10, 2017 17:50
addaleax pushed a commit to addaleax/ayo that referenced this pull request Oct 12, 2017
@stropitek @addaleax
test: increase test coverage of readline-interface
c554e09 
Adds coverage for:
- question callback
- history navigation
- bad historySize option
- multi-line output
- history is bound and most recent elements are preserved

PR-URL: nodejs/node#16062
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
targos pushed a commit that referenced this pull request Oct 18, 2017
@stropitek @targos
test: increase test coverage of readline-interface
497bceb 
Adds coverage for:
- question callback
- history navigation
- bad historySize option
- multi-line output
- history is bound and most recent elements are preserved

PR-URL: #16062
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BridgeAR BridgeAR BridgeAR approved these changes

@addaleax addaleax addaleax approved these changes

@targos targos targos approved these changes

Assignees

@lance lance

Labels

code-and-learn Issues related to the Code-and-Learn events and PRs submitted during the events. readline Issues and PRs related to the built-in readline module. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@stropitek @addaleax @lance @targos @BridgeAR @mscdex @MylesBorins @Trott @nodejs-github-bot