Skip to content

Commit

Permalink
Allow passing test location in cli
Browse files Browse the repository at this point in the history
* Add function to Parse filters + filter interface.  Goal is to Parse filters
  and pass to `interpretTaskModes`.

  Attempt to parse each filter with the format
  `<filename>:<line_number>`, while keeping mind that some files might
  have `:` in their filename.

  Throw an error when line range is provided.  Note that there is an edge
  case when a file has `:` and `-` in its filename.

* Add `FileSpec` type

  In `globTestSpecs`, the filters will be matched against the files, and
  if there is a location in the filter, it will be included with its file,
  in a `FileSpec: { path, testLocations? }`.

  I thought it would be a good idea to include the files with
  the location in the same object, instead of having them in their own
  attribute in the config and then matching based on the path or
  something.  But I ended up having to modify types everywhere to get this
  to work.

  I also had to modify `groupFilesByEnv` to include the `testLocations` in
  its output, and also modify pools to pass the test locations to
  `runFiles`.

  - Move the filter parsing inside `globTestSpecs` and revert related
    type changes

* Report disabled `includeTaskLocation`

  Add and handle new error `IncludeTasklocationDisabledError`

* Report file not found if a test location was specified

  - The reason for this is that we're using a more strict matching that
    requires full path to be specified

* Report wrong line number

  - Got rid of the confusing warning.

    Current behavior is that if user specifies a file with and without
    line numbers, it will not have an effect (e.g. `test.ts:3 test.ts`
    will match on line 3 only, not all files).
  • Loading branch information
mzhubail committed Oct 23, 2024
1 parent 9d9bad5 commit c65dbfc
Show file tree
Hide file tree
Showing 20 changed files with 275 additions and 79 deletions.
10 changes: 7 additions & 3 deletions packages/runner/src/collect.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { processError } from '@vitest/utils/error'
import { toArray } from '@vitest/utils'
import type { File, SuiteHooks } from './types/tasks'
import type { VitestRunner } from './types/runner'
import type { FileSpec, VitestRunner } from './types/runner'
import {
calculateSuiteHash,
createFileTask,
Expand All @@ -20,14 +20,17 @@ import { runSetupFiles } from './setup'
const now = globalThis.performance ? globalThis.performance.now.bind(globalThis.performance) : Date.now

export async function collectTests(
paths: string[],
specs: string[] | FileSpec[],
runner: VitestRunner,
): Promise<File[]> {
const files: File[] = []

const config = runner.config

for (const filepath of paths) {
for (const spec of specs) {
const filepath = typeof spec === 'string' ? spec : spec.filepath
const testLocations = typeof spec === 'string' ? undefined : spec.testLocations

const file = createFileTask(filepath, config.root, config.name, runner.pool)

runner.onCollectStart?.(file)
Expand Down Expand Up @@ -97,6 +100,7 @@ export async function collectTests(
interpretTaskModes(
file,
config.testNamePattern,
testLocations,
hasOnlyTasks,
false,
config.allowOnly,
Expand Down
13 changes: 8 additions & 5 deletions packages/runner/src/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { Awaitable } from '@vitest/utils'
import { getSafeTimers, shuffle } from '@vitest/utils'
import { processError } from '@vitest/utils/error'
import type { DiffOptions } from '@vitest/utils/diff'
import type { VitestRunner } from './types/runner'
import type { FileSpec, VitestRunner } from './types/runner'
import type {
Custom,
File,
Expand Down Expand Up @@ -495,10 +495,11 @@ export async function runFiles(files: File[], runner: VitestRunner): Promise<voi
}
}

export async function startTests(paths: string[], runner: VitestRunner): Promise<File[]> {
export async function startTests(specs: FileSpec[], runner: VitestRunner): Promise<File[]> {
const paths = specs.map(f => f.filepath)
await runner.onBeforeCollect?.(paths)

const files = await collectTests(paths, runner)
const files = await collectTests(specs, runner)

await runner.onCollected?.(files)
await runner.onBeforeRunFiles?.(files)
Expand All @@ -512,10 +513,12 @@ export async function startTests(paths: string[], runner: VitestRunner): Promise
return files
}

async function publicCollect(paths: string[], runner: VitestRunner): Promise<File[]> {
async function publicCollect(specs: string[] | FileSpec[], runner: VitestRunner): Promise<File[]> {
const paths = specs.map(f => typeof f === 'string' ? f : f.filepath)

await runner.onBeforeCollect?.(paths)

const files = await collectTests(paths, runner)
const files = await collectTests(specs, runner)

await runner.onCollected?.(files)
return files
Expand Down
5 changes: 5 additions & 0 deletions packages/runner/src/types/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ export interface VitestRunnerConfig {
diffOptions?: DiffOptions
}

export interface FileSpec {
filepath: string
testLocations: number[] | undefined
}

export type VitestRunnerImportSource = 'collect' | 'setup'

export interface VitestRunnerConstructor {
Expand Down
102 changes: 72 additions & 30 deletions packages/runner/src/utils/collect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,53 +6,95 @@ import type { File, Suite, TaskBase } from '../types/tasks'
* If any tasks been marked as `only`, mark all other tasks as `skip`.
*/
export function interpretTaskModes(
suite: Suite,
file: Suite,
namePattern?: string | RegExp,
testLocations?: number[] | undefined,
onlyMode?: boolean,
parentIsOnly?: boolean,
allowOnly?: boolean,
): void {
const suiteIsOnly = parentIsOnly || suite.mode === 'only'
const matchedLocations: number[] = []

suite.tasks.forEach((t) => {
// Check if either the parent suite or the task itself are marked as included
const includeTask = suiteIsOnly || t.mode === 'only'
if (onlyMode) {
if (t.type === 'suite' && (includeTask || someTasksAreOnly(t))) {
// Don't skip this suite
if (t.mode === 'only') {
const traverseSuite = (suite: Suite) => {
const suiteIsOnly = parentIsOnly || suite.mode === 'only'

suite.tasks.forEach((t) => {
// Check if either the parent suite or the task itself are marked as included
const includeTask = suiteIsOnly || t.mode === 'only'
if (onlyMode) {
if (t.type === 'suite' && (includeTask || someTasksAreOnly(t))) {
// Don't skip this suite
if (t.mode === 'only') {
checkAllowOnly(t, allowOnly)
t.mode = 'run'
}
}
else if (t.mode === 'run' && !includeTask) {
t.mode = 'skip'
}
else if (t.mode === 'only') {
checkAllowOnly(t, allowOnly)
t.mode = 'run'
}
}
else if (t.mode === 'run' && !includeTask) {
t.mode = 'skip'
if (t.type === 'test') {
if (namePattern && !getTaskFullName(t).match(namePattern)) {
t.mode = 'skip'
}

// Match test location against provided locations, only run if present
// in `testLocations`. Note: if `includeTaskLocations` is not enabled,
// all test will be skipped.
if (testLocations !== undefined && testLocations.length !== 0) {
if (t.location && testLocations?.includes(t.location.line)) {
t.mode = 'run'
matchedLocations.push(t.location.line)
}
else {
t.mode = 'skip'
}
}
}
else if (t.mode === 'only') {
checkAllowOnly(t, allowOnly)
t.mode = 'run'
else if (t.type === 'suite') {
if (t.mode === 'skip') {
skipAllTasks(t)
}
else {
traverseSuite(t)
// interpretTaskModes(t, namePattern, locationFilters, onlyMode, includeTask, allowOnly)
}
}
}
if (t.type === 'test') {
if (namePattern && !getTaskFullName(t).match(namePattern)) {
t.mode = 'skip'
})

// if all subtasks are skipped, mark as skip
if (suite.mode === 'run') {
if (suite.tasks.length && suite.tasks.every(i => i.mode !== 'run')) {
suite.mode = 'skip'
}
}
else if (t.type === 'suite') {
if (t.mode === 'skip') {
skipAllTasks(t)
}
else {
interpretTaskModes(t, namePattern, onlyMode, includeTask, allowOnly)
}

traverseSuite(file)

const nonMatching = testLocations?.filter(loc => !matchedLocations.includes(loc))
if (nonMatching && nonMatching.length !== 0) {
const message = nonMatching.length === 1
? `line ${nonMatching[0]}`
: `lines ${nonMatching.join(', ')}`

if (file.result === undefined) {
file.result = {
state: 'fail',
errors: [],
}
}
})

// if all subtasks are skipped, mark as skip
if (suite.mode === 'run') {
if (suite.tasks.length && suite.tasks.every(i => i.mode !== 'run')) {
suite.mode = 'skip'
if (file.result.errors === undefined) {
file.result.errors = []
}

file.result.errors.push(
processError(new Error(`No test found in ${file.name} in ${message}`)),
)
}
}

Expand Down
55 changes: 54 additions & 1 deletion packages/vitest/src/node/cli/cli-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import type { environments } from '../../integrations/env'
import { createVitest } from '../create'
import { registerConsoleShortcuts } from '../stdin'
import type { Vitest, VitestOptions } from '../core'
import { FilesNotFoundError, GitNotFoundError } from '../errors'
import { FilesNotFoundError, GitNotFoundError, IncludeTaskLocationDisabledError } from '../errors'
import type { UserConfig, VitestEnvironment, VitestRunMode } from '../types/config'
import type { WorkspaceSpec } from '../pool'
import { groupBy } from '../../utils/base'

export interface CliOptions extends UserConfig {
/**
Expand Down Expand Up @@ -109,6 +110,11 @@ export async function startVitest(
return ctx
}

if (e instanceof IncludeTaskLocationDisabledError) {
ctx.logger.printError(e, { verbose: false })
return ctx
}

process.exitCode = 1
ctx.logger.printError(e, { fullStack: true, type: 'Unhandled Error' })
ctx.logger.error('\n\n')
Expand Down Expand Up @@ -284,6 +290,53 @@ export function formatCollectedAsString(files: File[]) {
}).flat()
}

export function parseFilter(f: string) {
const colonIndex = f.indexOf(':')
if (colonIndex === -1) {
return { filename: f }
}

const [parsedFilename, lineNumber] = [
f.substring(0, colonIndex),
f.substring(colonIndex + 1),
]

if (lineNumber.match(/^\d+$/)) {
return {
filename: parsedFilename,
lineNumber: Number.parseInt(lineNumber),
}
}
else if (lineNumber.includes('-')) {
throw new Error('Range line numbers are not allowed')
}
else {
return { filename: f }
}
}

interface Filter {
filename: string
lineNumber?: undefined | number
}

export function groupFilters(filters: Filter[]) {
const groupedFilters_ = groupBy(filters, f => f.filename)
const groupedFilters = Object.fromEntries(Object.entries(groupedFilters_)
.map((entry) => {
const [filename, filters] = entry
const testLocations = filters.map(f => f.lineNumber)

return [
filename,
testLocations.filter(l => l !== undefined) as number[],
]
}),
)

return groupedFilters
}

const envPackageNames: Record<
Exclude<keyof typeof environments, 'node'>,
string
Expand Down
45 changes: 41 additions & 4 deletions packages/vitest/src/node/core.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { existsSync, promises as fs } from 'node:fs'
import type { Writable } from 'node:stream'
import { resolve } from 'node:path'
import type { ViteDevServer } from 'vite'
import { dirname, join, normalize, relative } from 'pathe'
import mm from 'micromatch'
Expand Down Expand Up @@ -28,12 +29,13 @@ import { VitestCache } from './cache'
import { WorkspaceProject } from './workspace'
import { VitestPackageInstaller } from './packageInstaller'
import { BlobReporter, readBlobs } from './reporters/blob'
import { FilesNotFoundError, GitNotFoundError } from './errors'
import { FilesNotFoundError, GitNotFoundError, IncludeTaskLocationDisabledError } from './errors'
import type { ResolvedConfig, UserConfig, VitestRunMode } from './types/config'
import type { Reporter } from './types/reporter'
import type { CoverageProvider } from './types/coverage'
import { resolveWorkspace } from './workspace/resolveWorkspace'
import type { TestSpecification } from './spec'
import { groupFilters, parseFilter } from './cli/cli-api'

const WATCHER_DEBOUNCE = 100

Expand Down Expand Up @@ -403,6 +405,13 @@ export class Vitest {
await this.globTestFiles(filters),
)

if (
!this.config.includeTaskLocation
&& files.some(spec => spec.testLocations && spec.testLocations.length !== 0)
) {
throw new IncludeTaskLocationDisabledError()
}

// if run with --changed, don't exit if no tests are found
if (!files.length) {
// Report coverage for uncovered files
Expand Down Expand Up @@ -1086,21 +1095,49 @@ export class Vitest {
}

public async globTestSpecs(filters: string[] = []) {
const parsedFilters = filters.map(f => parseFilter(f))
const testLocations = groupFilters(parsedFilters.map(
f => ({ ...f, filename: resolve(f.filename) }),
))

// Key is file and val sepcifies whether we have matched this file with testLocation
const testLocHasMatch: { [f: string]: boolean } = {}

const files: WorkspaceSpec[] = []
await Promise.all(this.projects.map(async (project) => {
const { testFiles, typecheckTestFiles } = await project.globTestFiles(filters)
const { testFiles, typecheckTestFiles } = await project.globTestFiles(
parsedFilters.map(f => f.filename),
)

testFiles.forEach((file) => {
const pool = getFilePoolName(project, file)
const spec = project.createSpec(file, pool)
const loc = testLocations[file]
testLocHasMatch[file] = true

const spec = project.createSpec(file, pool, loc)
this.ensureSpecCached(spec)
files.push(spec)
})
typecheckTestFiles.forEach((file) => {
const spec = project.createSpec(file, 'typescript')
const loc = testLocations[file]
testLocHasMatch[file] = true

const spec = project.createSpec(file, 'typescript', loc)
this.ensureSpecCached(spec)
files.push(spec)
})
}))

Object.entries(testLocations).forEach(([filepath, loc]) => {
if (loc.length !== 0 && !testLocHasMatch[filepath]) {
const rel = relative(this.config.dir || this.config.root, filepath)

this.logger.printError(new Error(`Couldn\'t find file "${rel}".\n`
+ 'Note when specifying the test location you have to specify the full test filename.',
))
}
})

return files
}

Expand Down
8 changes: 8 additions & 0 deletions packages/vitest/src/node/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,11 @@ export class GitNotFoundError extends Error {
super('Could not find Git root. Have you initialized git with `git init`?')
}
}

export class IncludeTaskLocationDisabledError extends Error {
code = 'VITEST_INCLUDE_TASK_LOCATION_DISABLED'

constructor() {
super('Recieved line number filters while `includeTaskLocation` option is disabled')
}
}
Loading

0 comments on commit c65dbfc

Please sign in to comment.