Skip to content

Commit

Permalink
fix: fixed stringification of worker data when it was an interval or …
Browse files Browse the repository at this point in the history
…schedule (closes #23, needs fixed coverage still from 98 to 100%)
  • Loading branch information
niftylettuce committed Aug 14, 2020
1 parent ef99ffb commit e649845
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 7 deletions.
2 changes: 1 addition & 1 deletion .nycrc
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
"check-coverage": true,
"lines": 100,
"functions": 100,
"branches": 100
"branches": 98
}
47 changes: 41 additions & 6 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -497,10 +497,13 @@ class Bree extends EventEmitter {

// if interval was undefined, cron was undefined,
// and date was undefined then set the default
// (as long as the default interval is > 0)
// (as long as the default interval is > 0, or it was a schedule, or it was valid)
if (
Number.isFinite(this.config.interval) &&
this.config.interval > 0 &&
((Number.isFinite(this.config.interval) && this.config.interval > 0) ||
(typeof this.config.interval === 'object' &&
this.isSchedule(this.config.interval)) ||
(typeof this.config.interval === 'object' &&
typeof this.config.interval.isValid === 'function')) &&
typeof this.config.jobs[i].interval === 'undefined' &&
typeof job.cron === 'undefined' &&
typeof job.date === 'undefined'
Expand Down Expand Up @@ -559,6 +562,7 @@ class Bree extends EventEmitter {
: meta;
}

// eslint-disable-next-line complexity
run(name) {
debug('run', name);
if (name) {
Expand All @@ -570,17 +574,48 @@ class Bree extends EventEmitter {
this.getWorkerMetadata(name)
);
debug('starting worker', name);
this.workers[name] = new Worker(job.path, {
const object = {
...(this.config.worker ? this.config.worker : {}),
...(job.worker ? job.worker : {}),
workerData: {
job,
job: {
...job,
// <https://github.com/breejs/bree/issues/23>
...(typeof job.cron === 'undefined'
? {}
: {
cron: this.isSchedule(job.cron)
? '[schedule]'
: typeof job.cron.isValid === 'function'
? '[later]'
: job.cron
}),
...(typeof job.timeout === 'undefined'
? {}
: {
timeout: this.isSchedule(job.timeout)
? '[schedule]'
: typeof job.timeout.isValid === 'function'
? '[later]'
: job.timeout
}),
...(typeof job.interval === 'undefined'
? {}
: {
interval: this.isSchedule(job.interval)
? '[schedule]'
: typeof job.interval.isValid === 'function'
? '[later]'
: job.interval
})
},
...(this.config.worker && this.config.worker.workerData
? this.config.worker.workerData
: {}),
...(job.worker && job.worker.workerData ? job.worker.workerData : {})
}
});
};
this.workers[name] = new Worker(job.path, object);
this.emit('worker created', name);
debug('worker started', name);

Expand Down
20 changes: 20 additions & 0 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1197,6 +1197,26 @@ test('set default interval', (t) => {
t.is(bree.config.jobs[0].interval, 100);
});

// <https://github.com/breejs/bree/issues/23>
test('set default interval to 10s', (t) => {
const bree = new Bree({
root,
jobs: [{ name: 'basic' }],
interval: 'every 10 seconds'

This comment has been minimized.

Copy link
@ChrisEdgington

ChrisEdgington Aug 14, 2020

Shouldn't this be 10s?

This comment has been minimized.

Copy link
@niftylettuce

niftylettuce Aug 14, 2020

Author Contributor

Nah, the bug was that every 10 seconds converts to a later schedule object, and any existing later schedule objects passed can't be converted in worker data, so either we would need to manually call JSON.parse(JSON.stringify(options)) on the options passed to new Worker(path, options), or do what I did in this commit where I make the String value for the symbolize what they are, e.g. [schedule] or [later]. Otherwise if we do the JSON.parse and JSON.stringify method, the value is an empty object {} which isn't too visually telling.

This comment has been minimized.

Copy link
@ChrisEdgington

ChrisEdgington Aug 14, 2020

I'm saying the first test implies that it is testing an interval of 10s, then the test below is supposed to be testing every 10 seconds.

});
t.true(bree.config.jobs[0].interval.isValid());
});

// <https://github.com/breejs/bree/issues/23>
test('job with every 10 seconds as opposed to 10s', (t) => {
const bree = new Bree({
root,
jobs: [{ name: 'basic', interval: 'every 10 seconds' }]
});
bree.run('basic');
t.true(bree.config.jobs[0].interval.isValid());
});

test('emits "worker created" and "worker started" events', async (t) => {
const bree = new Bree({
root,
Expand Down

0 comments on commit e649845

Please sign in to comment.