Skip to content

Conversation

@devsnek
Copy link
Member

@devsnek devsnek commented Nov 4, 2017

see nodejs/node#16745 for more useful information

@phillipj
Copy link
Member

phillipj commented Nov 4, 2017

Awesome, thanks!

A test verify this works as expected in the future would be much appreciated.

The labels tests are quite extensive, don't hesitate to ask if you got any questions navigating those tests or like some thoughts on how / where to add a test.

@devsnek
Copy link
Member Author

devsnek commented Nov 4, 2017

node-js-subsystem-labels.test.js will fail expectedly

@phillipj
Copy link
Member

phillipj commented Nov 6, 2017

Thanks for adding a test 👍

I'm seeing this when running npm test:

test/unit/node-js-subsystem-labels.test.js ............ 7/8
  label: lib oddities
  not ok should be equivalent
    +++ found
    --- wanted
     [
       "debugger"
    -  "http"
    +  "_http_agent"
    +  "_http_client"
    +  "_http_common"
    +  "_http_incoming"
    +  "_http_outgoing"
    +  "_http_server"
       "timers"
    -  "stream"
    -  "tls"
    +  "_stream_duplex"
    +  "_stream_passthrough"
    +  "_stream_readable"
    +  "_stream_transform"
    +  "_stream_wrap"
    +  "_stream_writable"
    +  "_tls_common"
    +  "_tls_legacy"
    +  "_tls_wrap"
       "lib / src"
       "process"
       "net"
       "tools"
    at:
      line: 39
      column: 5
      file: test/unit/node-js-subsystem-labels.test.js
      type: Test
      function: tap.test
    stack: |
      Test.tap.test (test/unit/node-js-subsystem-labels.test.js:39:5)
      Object.<anonymous> (test/unit/node-js-subsystem-labels.test.js:7:5)
    source: |
      t.same(labels, [

...

  221 passing (4s)
  1 failing

standard: Use JavaScript Standard Style (http://standardjs.com)
  /github/github-bot/lib/node-labels.js:95:42: Unexpected trailing comma.
npm ERR! Test failed.  See above for more details.

We need to make sure all previous tests + the newly added one passes, before this can be merged into master.

@devsnek
Copy link
Member Author

devsnek commented Nov 6, 2017

I guess now that we have those stubs and the folders are in internal this PR is actually not not needed

@devsnek devsnek closed this Nov 6, 2017
@devsnek devsnek deleted the pr-16745 branch February 2, 2018 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants