Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/vest/src/core/context/SuiteContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { assign, TinyState, tinyState, DynamicValue } from 'vest-utils';

import { Modes } from '../../hooks/optional/Modes';
import { SuiteModifiers } from '../../suite/SuiteTypes';
import { TFieldName, TSchema } from '../../suiteResult/SuiteResultTypes';
import {
TFieldName,
TGroupName,
TSchema,
} from '../../suiteResult/SuiteResultTypes';
import { TIsolateTest } from '../isolate/IsolateTest/IsolateTest';

export const SuiteContext = createCascade<CTXType>((ctxRef, parentContext) => {
Expand Down Expand Up @@ -31,7 +35,7 @@ type CTXType = {
skipped?: boolean;
omitted?: boolean;
schema: TSchema;
modifiers: SuiteModifiers<TFieldName>;
modifiers: SuiteModifiers<TFieldName, TGroupName>;
};

export function useCurrentTest(msg?: string) {
Expand Down
139 changes: 123 additions & 16 deletions packages/vest/src/hooks/focused/focused.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import {
OneOrMoreOf,
noop,
Nullable,
isArray,
isNotEmpty,
isObject,
isStringValue,
makeBrand,
makeResult,
Expand All @@ -13,54 +15,78 @@ import {
import { IsolateSelectors, TIsolate, Isolate } from 'vestjs-runtime';

import { VestIsolateType } from '../../core/isolate/VestIsolateType';
import { TFieldName } from '../../suiteResult/SuiteResultTypes';
import { TFieldName, TGroupName } from '../../suiteResult/SuiteResultTypes';

import { FocusModes } from './FocusedKeys';

export type FieldExclusion<F extends string = TFieldName> = Maybe<
OneOrMoreOf<F>
>;
export type GroupExclusion<G extends string = TGroupName> = Maybe<
OneOrMoreOf<G>
>;
export type FocusMatch<
F extends string = TFieldName,
G extends string = TGroupName,
> =
| FieldExclusion<F>
| {
fields?: FieldExclusion<F>;
groups?: GroupExclusion<G>;
};

export type TIsolateFocused = TIsolate<IsolateFocusedPayload>;

type IsolateFocusedPayload = {
focusMode: FocusModes;
match: FieldExclusion<TFieldName>;
fields: TFieldName[];
groups: TGroupName[];
matchAll: boolean;
};

export function IsolateFocused(
focusMode: FocusModes,
match?: true | FieldExclusion<string>,
match?: true | FocusMatch<string, string>,
): TIsolateFocused {
const matchedFields = asArray(match)
.filter(isStringValue)
.map(makeBrand<TFieldName>) as TFieldName[];
const normalizedMatch = normalizeMatch(match);
const matchedFields = normalizedMatch.fields;
const matchedGroups = normalizedMatch.groups;

return Isolate.create(VestIsolateType.Focused, noop, {
fields: matchedFields,
focusMode,
match: matchedFields,
matchAll: match === true,
groups: matchedGroups,
matchAll: normalizedMatch.matchAll,
});
}

export class FocusSelectors {
static isFocusMatch(
focus: Nullable<TIsolateFocused>,
fieldName?: TFieldName,
groupName?: TGroupName,
): Result<boolean> {
return makeResult.Ok(hasFocus(focus, fieldName, groupName).unwrap());
}

static isSkipFocused(
focus: Nullable<TIsolateFocused>,
fieldName?: TFieldName,
groupName?: TGroupName,
): Result<boolean> {
return makeResult.Ok(
focus?.data.focusMode === FocusModes.SKIP &&
(hasFocus(focus, fieldName).unwrap() || focus.data.matchAll === true),
hasFocus(focus, fieldName, groupName).unwrap(),
);
}
static isOnlyFocused(
focus: Nullable<TIsolateFocused>,
fieldName?: TFieldName,
groupName?: TGroupName,
): Result<boolean> {
return makeResult.Ok(
focus?.data.focusMode === FocusModes.ONLY &&
hasFocus(focus, fieldName).unwrap(),
hasFocus(focus, fieldName, groupName).unwrap(),
);
}

Expand All @@ -76,7 +102,7 @@ export class FocusSelectors {
*
* only('username');
*/
export function only(match: FieldExclusion<string> | false) {
export function only(match: FocusMatch<string, string> | false) {
return IsolateFocused(FocusModes.ONLY, defaultMatch(match).unwrap());
}
/**
Expand All @@ -86,23 +112,104 @@ export function only(match: FieldExclusion<string> | false) {
*
* skip('username');
*/
export function skip(match: FieldExclusion<string> | boolean) {
export function skip(match: FocusMatch<string, string> | boolean) {
return IsolateFocused(FocusModes.SKIP, defaultMatch(match).unwrap());
}

function defaultMatch(
match: FieldExclusion<string> | boolean,
): Result<FieldExclusion<string> | true> {
match: FocusMatch<string, string> | boolean,
): Result<FocusMatch<string, string> | true> {
return makeResult.Ok(match === false ? [] : match);
}

// eslint-disable-next-line complexity
function hasFocus(
focus: Nullable<TIsolateFocused>,
fieldName?: TFieldName,
groupName?: TGroupName,
): Result<boolean> {
if (focus?.data.matchAll) {
return makeResult.Ok(true);
}

const hasFieldMatch =
isNotEmpty(focus?.data.fields) &&
(fieldName ? (focus?.data.fields?.includes(fieldName) ?? true) : true);
const hasGroupMatch =
isNotEmpty(focus?.data.groups) &&
(groupName ? (focus?.data.groups?.includes(groupName) ?? true) : true);

return makeResult.Ok(
isNotEmpty(focus?.data.match) &&
(fieldName ? (focus?.data.match?.includes(fieldName) ?? true) : true),
(hasFieldMatch || hasGroupMatch) &&
(fieldName || groupName
? true
: isNotEmpty(focus?.data.fields) || isNotEmpty(focus?.data.groups)),
);
}

function normalizeMatch(match: true | FocusMatch<string, string> | undefined): {
fields: TFieldName[];
groups: TGroupName[];
matchAll: boolean;
} {
if (match === true) {
return {
fields: [],
groups: [],
matchAll: true,
};
}

let fieldsSource: FocusMatch<string, string> | undefined = match;
let groupsSource: GroupExclusion<string> | undefined;

if (isFocusGroupConfig(match)) {
fieldsSource = match?.fields;
groupsSource = match?.groups;
}

const fields = asArray(fieldsSource)
.filter(isStringValue)
.map(makeBrand<TFieldName>) as TFieldName[];
const groups = asArray(groupsSource)
.filter(isStringValue)
.map(makeBrand<TGroupName>) as TGroupName[];

return {
fields,
groups,
matchAll: false,
};
}

function isFocusGroupConfig(
match: FocusMatch<string, string> | undefined,
): match is {
fields?: FieldExclusion<string>;
groups?: GroupExclusion<string>;
} {
const candidate = getFocusGroupCandidate(match);
return !!candidate && hasFocusGroupKeys(candidate);
}

function getFocusGroupCandidate(
match: FocusMatch<string, string> | undefined,
): Record<string, unknown> | null {
if (!isObject(match)) {
return null;
}

if (isArray(match)) {
return null;
}

if (isStringValue(match)) {
return null;
}

return match;
}

function hasFocusGroupKeys(match: Record<string, unknown>): boolean {
return 'fields' in match || 'groups' in match;
}
5 changes: 3 additions & 2 deletions packages/vest/src/hooks/focused/useHasOnliedTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { isNotNullish } from 'vest-utils';
import { TIsolate, Walker } from 'vestjs-runtime';

import { TIsolateTest } from '../../core/isolate/IsolateTest/IsolateTest';
import { TFieldName } from '../../suiteResult/SuiteResultTypes';
import { TFieldName, TGroupName } from '../../suiteResult/SuiteResultTypes';

import { FocusSelectors } from './focused';

Expand All @@ -12,12 +12,13 @@ import { FocusSelectors } from './focused';
export function useHasOnliedTests(
testObject: TIsolateTest,
fieldName?: TFieldName,
groupName?: TGroupName,
): boolean {
return isNotNullish(
Walker.findClosest(testObject, (child: TIsolate) => {
if (!FocusSelectors.isIsolateFocused(child)) return false;

return FocusSelectors.isOnlyFocused(child, fieldName).unwrap();
return FocusSelectors.isOnlyFocused(child, fieldName, groupName).unwrap();
}),
);
Comment on lines 12 to 23
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The useHasOnliedTests function, as modified in this PR, checks if the given testObject (or an ancestor) is only focused. However, in useIsExcludedByInclusion, this function is called after it has already been determined that the testObject is not explicitly only focused. This means useHasOnliedTests will always return false in that context.

To correctly implement the suite.only behavior (where all non-focused tests are excluded unless explicitly included), useIsExcludedByInclusion needs to know if any only isolates are active in the suite globally, not if the current test itself is only focused. The current implementation of useHasOnliedTests does not serve this purpose.

I recommend refactoring useHasOnliedTests to check for the global presence of only isolates in the suite. Alternatively, introduce a new helper function for this global check and use it in useIsExcludedByInclusion.

Here's a suggestion to introduce a new global check function and keep the existing useHasOnliedTests for its specific (though currently unused in useIsExcludedByInclusion) purpose:

import { isNotNullish } from 'vest-utils';
import { TIsolate, Walker, VestRuntime } from 'vestjs-runtime';
import { FocusModes } from './FocusedKeys';

import { TIsolateTest } from '../../core/isolate/IsolateTest/IsolateTest';
import { TFieldName, TGroupName } from '../../suiteResult/SuiteResultTypes';

import { FocusSelectors } from './focused';

/**
 * Checks if there are any 'only' focused isolates active in the current suite context.
 * This function performs a global check across the entire suite isolate tree.
 */
export function hasAnyOnlyIsolatesInSuite(): boolean {
  const currentSuiteIsolate = VestRuntime.useAvailableRoot();
  if (!currentSuiteIsolate) return false;

  return Walker.some(currentSuiteIsolate, (isolate: TIsolate) => {
    if (!FocusSelectors.isIsolateFocused(isolate)) return false;
    return isolate.data.focusMode === FocusModes.ONLY;
  });
}

/**
 * Checks if the given testObject (or any of its ancestors) is 'only' focused
 * for the specified fieldName and groupName.
 */
export function useHasOnliedTests(
  testObject: TIsolateTest,
  fieldName?: TFieldName,
  groupName?: TGroupName,
): boolean {
  return isNotNullish(
    Walker.findClosest(testObject, (child: TIsolate) => {
      if (!FocusSelectors.isIsolateFocused(child)) return false;

      return FocusSelectors.isOnlyFocused(child, fieldName, groupName).unwrap();
    }),
  );
}

}
51 changes: 36 additions & 15 deletions packages/vest/src/hooks/focused/useIsExcluded.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,36 +13,57 @@ import { useHasOnliedTests } from './useHasOnliedTests';
function useClosestMatchingFocus(
testObject: TIsolateTest,
): Result<Nullable<TIsolateFocused>> {
const { fieldName } = VestTest.getData(testObject);
const groupName = VestTest.getGroupName(testObject);

return makeResult.Ok(
Walker.findClosest(testObject, (child: TIsolate) => {
if (!FocusSelectors.isIsolateFocused(child)) return false;

const { fieldName } = VestTest.getData(testObject);

return child.data.match?.includes(fieldName) || child.data.matchAll;
return FocusSelectors.isFocusMatch(
child as TIsolateFocused,
fieldName,
groupName,
).unwrap();
}),
);
}

export function useIsExcluded(testObject: TIsolateTest): boolean {
const { fieldName } = VestTest.getData(testObject);

if (useIsExcludedIndividually()) return true;
const inclusion = useInclusion();
return useIsExcludedByFocus(testObject);
}

function useIsExcludedByFocus(testObject: TIsolateTest): boolean {
const { fieldName } = VestTest.getData(testObject);
const groupName = VestTest.getGroupName(testObject);
const focusMatch = useClosestMatchingFocus(testObject).unwrap();

// if test is skipped
// no need to proceed
if (FocusSelectors.isSkipFocused(focusMatch).unwrap()) return true;
const isTestIncluded = FocusSelectors.isOnlyFocused(focusMatch).unwrap();
// if field is only'ed
if (isTestIncluded) return false;
if (FocusSelectors.isSkipFocused(focusMatch, fieldName, groupName).unwrap()) {
return true;
}

if (FocusSelectors.isOnlyFocused(focusMatch, fieldName, groupName).unwrap()) {
// if field is only'ed
return false;
}

return useIsExcludedByInclusion(testObject, fieldName, groupName);
}

function useIsExcludedByInclusion(
testObject: TIsolateTest,
fieldName: string,
groupName?: string,
): boolean {
// If there is _ANY_ `only`ed test (and we already know this one isn't) return true
if (useHasOnliedTests(testObject)) {
// Check if inclusion rules for this field (`include` hook)
return !dynamicValue(inclusion[fieldName], testObject);
if (!useHasOnliedTests(testObject, fieldName, groupName)) {
return false;
}

// We're done here. This field is not excluded
return false;
const inclusion = useInclusion();
// Check if inclusion rules for this field (`include` hook)
return !dynamicValue(inclusion[fieldName], testObject);
}
Comment on lines +56 to 69
Copy link
Contributor

Choose a reason for hiding this comment

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

high

The logic within useIsExcludedByInclusion is currently inverted, leading to incorrect behavior when suite.only is active.

Here's the breakdown:

  1. useIsExcludedByFocus determines that the current testObject is not explicitly only focused, and then calls useIsExcludedByInclusion.
  2. Inside useIsExcludedByInclusion, the condition if (!useHasOnliedTests(testObject, fieldName, groupName)) is evaluated. Since useHasOnliedTests (as currently implemented) checks if this specific test is only focused, and we already know it's not, useHasOnliedTests will return false.
  3. Consequently, !useHasOnliedTests(...) becomes true, causing the function to immediately return false.

This means that if only mode is active, but the current test is not among the only focused tests, it will not be excluded. This breaks the fundamental behavior of suite.only, which should exclude all tests not explicitly focused.

To fix this, useIsExcludedByInclusion should check for the global presence of any only isolates in the suite (using the hasAnyOnlyIsolatesInSuite function proposed in the useHasOnliedTests.ts comment). If global only mode is active, and the current test is not explicitly only focused (which is the precondition for calling this function), then the test should be excluded unless an include hook explicitly includes it.

function useIsExcludedByInclusion(
  testObject: TIsolateTest,
  fieldName: string,
  groupName?: string,
): boolean {
  // If there are *any* 'only' tests active in the suite, and this test is not
  // explicitly 'only' focused (as determined by useIsExcludedByFocus),
  // then this test should be excluded, unless it's explicitly included by an 'include' hook.
  if (hasAnyOnlyIsolatesInSuite()) { // Use the new global check function
    const inclusion = useInclusion();
    // Check if inclusion rules for this field (`include` hook)
    return !dynamicValue(inclusion[fieldName], testObject);
  }

  // If no 'only' tests are active globally, then this test is not excluded by 'only' logic.
  return false;
}
Suggested change
function useIsExcludedByInclusion(
testObject: TIsolateTest,
fieldName: string,
groupName?: string,
): boolean {
// If there is _ANY_ `only`ed test (and we already know this one isn't) return true
if (useHasOnliedTests(testObject)) {
// Check if inclusion rules for this field (`include` hook)
return !dynamicValue(inclusion[fieldName], testObject);
if (!useHasOnliedTests(testObject, fieldName, groupName)) {
return false;
}
// We're done here. This field is not excluded
return false;
const inclusion = useInclusion();
// Check if inclusion rules for this field (`include` hook)
return !dynamicValue(inclusion[fieldName], testObject);
}
function useIsExcludedByInclusion(
testObject: TIsolateTest,
fieldName: string,
groupName?: string,
): boolean {
// If there are *any* 'only' tests active in the suite, and this test is not
// explicitly 'only' focused (as determined by useIsExcludedByFocus),
// then this test should be excluded, unless it's explicitly included by an 'include' hook.
if (hasAnyOnlyIsolatesInSuite()) {
const inclusion = useInclusion();
// Check if inclusion rules for this field (`include` hook)
return !dynamicValue(inclusion[fieldName], testObject);
}
// If no 'only' tests are active globally, then this test is not excluded by 'only' logic.
return false;
}

9 changes: 5 additions & 4 deletions packages/vest/src/suite/SuiteTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { StandardSchemaV1 } from 'vest-utils/standardSchemaSpec';

import { Subscribe } from '../core/VestBus/VestBus';
import { TIsolateSuite } from '../core/isolate/IsolateSuite/IsolateSuite';
import { FieldExclusion } from '../hooks/focused/focused';
import { FocusMatch } from '../hooks/focused/focused';
import {
SuiteResult,
TFieldName,
Expand Down Expand Up @@ -74,14 +74,15 @@ type AfterMethods<
AfterMethods<F, G, T, S>,
[fieldName: F | string, callback: CB]
>;
focus: CB<AfterMethods<F, G, T, S>, [config: SuiteModifiers<F>]>;
focus: CB<AfterMethods<F, G, T, S>, [config: SuiteModifiers<F, G>]>;
only: CB<AfterMethods<F, G, T, S>, [match: FocusMatch<F, G>]>;
run: (
...args: S extends undefined
? Parameters<T>
: [data: InferSchemaData<S>, ...args: any[]]
) => SuiteResult<F, G>;
};

export type SuiteModifiers<F extends TFieldName> = {
only?: FieldExclusion<F> | FieldExclusion<string>;
export type SuiteModifiers<F extends TFieldName, G extends TGroupName> = {
only?: FocusMatch<F, G> | FocusMatch<string, string>;
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`suite.focus: only > focus return value > should be the rest of the suite methods 1`] = `
exports[`suite.only > only return value > should be the rest of the suite methods 1`] = `
{
"afterEach": [Function],
"afterField": [Function],
Expand Down
Loading
Loading