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

fix: options behaviour in findAPortNotInUse and findAPortInUse #56

Conversation

alexander-akait
Copy link
Contributor

@alexander-akait alexander-akait commented Nov 30, 2017

@laggingreflex
Include:

  1. Fix options behaviour in findAPortNotInUse and findAPortInUse
  2. Improve tests and increase coverage (99.07%)
  3. Update async to latest (import perf)

@alexander-akait alexander-akait force-pushed the fix-options-behavior-in-findAPort-functions branch 2 times, most recently from e49f862 to d90a5db Compare November 30, 2017 11:18
@alexander-akait alexander-akait force-pushed the fix-options-behavior-in-findAPort-functions branch from d90a5db to 4fc93fe Compare November 30, 2017 11:24
@alexander-akait
Copy link
Contributor Author

@laggingreflex also i can rewrite tests using jest (in next PR), it is allow mocking and do tests more simple and stable

@laggingreflex
Copy link
Collaborator

Hi @evilebottnawi Extremely sorry for delay.

  1. Fix options behaviour in findAPortNotInUse and findAPortInUse

I don't see findAPortNotInUse or findAPortInUse functions ever accepting an options argument. Only the checkPortStatus function does so. Although findAPortNotInUse doesn't throw when passed options it doesn't work either.

You said that it's being used this way in browser-sync, I'm not sure why they used it that way.

This could be a great feature, but I just wanna be clear on whether this is fixing a bug which is causing other projects dependent on it fail, or it's just a new feature?


Another thing was that I couldn't run tests. Got an error:

  √ checkPortStatus - free (with host in options argument) (401ms)
C:\...\portscanner\test.js:191
      t.is(t._expr(t._capt(t._capt(error, 'arguments/0/object').code, 'arguments/0'), {
                                                               ^

TypeError: Cannot read property 'code' of null
    at C:\...\portscanner\test.js:191:64
    at Socket.<anonymous> (C:\...\portscanner\lib\portscanner.js:132:5)
    at emitOne (events.js:125:13)
    at Socket.emit (events.js:221:7)
    at TCP._handle.close [as _onclose] (net.js:558:12)
  √ checkPortStatus - free (402ms)
TypeError: Cannot read property 'stack' of undefined
    at error (C:\...\portscanner\node_modules\ava\cli.js:51:20)
    at runCallback (timers.js:800:20)
    at tryOnImmediate (timers.js:762:5)
    at processImmediate [as _immediateCallback] (timers.js:733:5)
From previous event:
    at Object.<anonymous> (C:\...\portscanner\node_modules\ava\cli.js:216:34)
    at Module._compile (module.js:641:30)
    at Object.Module._extensions..js (module.js:652:10)
    ...

I hope this isn't just my OS (Windows). I've set up CircleCI.


I'm in favour of Jest.


Lastly, I wanna set up something like don't-break before adding any new features so please give me some time for that as well.

@alexander-akait
Copy link
Contributor Author

alexander-akait commented May 11, 2018

/cc @laggingreflex repo are abadoned?

@laggingreflex laggingreflex merged commit 7ffd84c into baalexander:master May 14, 2018
@laggingreflex
Copy link
Collaborator

I tested it on *nix and the tests are passing. I also tested against browser-sync whose tests are passing too. So this should be good to release.

Sorry about the delay again.

@laggingreflex
Copy link
Collaborator

Merged, 2.2.0 released.

@alexander-akait alexander-akait deleted the fix-options-behavior-in-findAPort-functions branch May 14, 2018 10:22
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