Add tags to journeys + filtering #60
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
| @@ -56,7 +56,8 @@ process.env.DEBUG = program.debug || ''; | |||
| headless: program.headless, | |||
| screenshots: program.screenshots, | |||
| dryRun: program.dryRun, | |||
| journeyName: program.journeyName, | |||
| journeyNames: program.journeyNames, | |||
| journeyTags: program.journeyTags, | |||
There was a problem hiding this comment.
Should we consider renaming these to make it clear that these are for filtering? Or maybe the original proposal that had include-tags, exclude-tags
There was a problem hiding this comment.
Ah, that's right we discussed this. I'm glad to rename it as you discussed. --includeTags and --includeJourneys which will accept the globs. I'll also include an exclude variant.
| @@ -3,7 +3,7 @@ import { grey, cyan, dim, italic } from 'kleur/colors'; | |||
| import { now } from '../helpers'; | |||
|
|
|||
| const defaultFd = process.stdout.fd; | |||
| let logger = new SonicBoom({ fd: defaultFd }); | |||
| let logger = new SonicBoom({ fd: defaultFd, sync: true }); | |||
There was a problem hiding this comment.
Why is the sync option necessary here? adding a comment in the code would be useful
There was a problem hiding this comment.
Ah, good catch, it's not I need to remove this. I was trying to see if that would fix #69 , but it did not. I'll revert this change.
| } | ||
|
|
||
| tagsMatch(globs: string[]): boolean { | ||
| return globs.some(glob => match(this.options.tags, glob).length > 0); |
There was a problem hiding this comment.
nit: might be better to refactor this into a function like:
const isMatch = (list, globs) => globs.some(glob => match(list,glob).length>0)
|
|
||
| for (const j of matchingJourneys) { | ||
| const journeyResult = await this.runJourney(j, options); | ||
| result[j.options.name] = journeyResult; |
There was a problem hiding this comment.
nit: I would match the journey in this for, instead of having a separate filter.
Something like
if(j.matches(...)){
// run journey
}
There was a problem hiding this comment.
Hmmm, I initially did that, but thought it was cleaner to hoist it out, It's not a big deal, glad to move it back
| 'only run the journey with the given name(s), or glob matches' | ||
| ) | ||
| .option( | ||
| '--journey-tags <name>...', |
| @@ -26,7 +26,8 @@ export async function run(options: RunOptions) { | |||
| headless: options.headless ?? cliArgs.headless, | |||
| screenshots: options.screenshots ?? cliArgs.screenshots, | |||
| dryRun: options.dryRun ?? cliArgs.dryRun, | |||
| journeyName: options.journeyName ?? cliArgs.journeyName, | |||
| journeyNames: options.journeyNames ?? cliArgs.journeyName, | |||
There was a problem hiding this comment.
As our runner is supposed to be running only journeys for now. should we just keep it simple and call name and tags instead? It feels redundant
|
Closing in favor of #300 |
Fixes #26 adds tags as metadata to journeys that can be filtered on. These tags will be searchable / filterable in the Uptime UI