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

feat(): readyNotice as function #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

shmilky
Copy link

@shmilky shmilky commented Jul 9, 2020

Support passing a method as a readyNotice that will be called instead of listening to text message

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.5%) to 96.316% when pulling bc10b12 on shmilky:master into 94334b0 on osher:master.

Copy link
Owner

@osher osher left a comment

Choose a reason for hiding this comment

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

mm. that gets to be lots of new code paths.

readme also needs updating.

please- change the merge target from master to readyNoticeFn
I'll add tests & readme there :)

@@ -1,6 +1,6 @@
{
"name": "e2e-helper",
"version": "0.9.4",
"version": "0.10.0",
Copy link
Owner

@osher osher Jul 9, 2020

Choose a reason for hiding this comment

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

its fully compatible with prev versions.
Since this version starts with 0, by semver rules the minor is acting like a major and (~ and ^ behave the same)
leave version bumping to me

@@ -183,7 +184,7 @@ function initCtx(options) {
, " - logPath - string, optional - path to logfile. default: './e2e.log'"
, " - timeout - integer, optional - timeout for server setup, default: " + defaults.timeout
, " - slow - integer, optional - slow bar indicator for server setup, default: " + defaults.timeout
, " - readyNotice - string, optional - message to expect on service output that"
, " - readyNotice - string or function, optional - message to expect on service output that"
Copy link
Owner

Choose a reason for hiding this comment

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

replace lines 187 and 188 with:

        , " - readyNotice - optional string or a function,"
        , "    - string:  message to expect on service output that indicates the service is ready."
        , "    - function: is passed the usual err-1st callback to call when the service is ready, or to pass an error when such error occurs."
        , "       may also return a Promise instread"
        , "    default: the string '" + defaults.readyNotice + "'"

@param {mocha.Test} test - the test context that implements .timetout(n), .slow(n) ....
@param {callback} done - callback
@param {function} done - callback
Copy link
Owner

Choose a reason for hiding this comment

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

not any function. an err-1st callback - hence callback.

}
})
}

Copy link
Owner

@osher osher Jul 9, 2020

Choose a reason for hiding this comment

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

if ('string' == typeof ctx.readyNotice) {
    const readyNotice = ctx.readyNotice
    ctx.readyNotice = () => new Promise(done => child.stdout.on('data', (data) => ~data.indexOf(readyNotice) && done()))
}

var promise = ctx.readyNotice((err) => {
    if (!done) return
    if (!err) ctx.console.log('service started: %s', ctx.args.join(' '))
    done(err)
    done = null
})
promise && promise.then && promise.catch((err) => err || new Error('ready hook rejection')).then((err) => {
    done(err instasnceof Error ? err : null)
    done = null
})

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.

None yet

4 participants