fix(main): fixed the hooks implementation

.changeset/thin-gorillas-attend.md

@@ -0,0 +1,5 @@
+---
+"quickpickle": patch
+---
+
+Fixed errors in the hook implementations.

packages/main/src/hooks.ts

@@ -1,11 +1,10 @@
 import { isFunction, isString, isObject, concat } from 'lodash-es'
-import { tagsFunction } from './tags';
+import { normalizeTags } from './tags';
 
 interface Hook {
   name: string;
   f: (state: any) => Promise<any> | any;
-  tagsFunction: (tags: string[]) => boolean;
-  tags?: string;
+  tags?: string|string[];
 }
 
 interface HookCollection {
@@ -33,48 +32,47 @@ const hookNames: { [key: string]: string } = {
 const applyHooks = async (hooksName: string, state: any): Promise<any> => {
   const hooks = allHooks[hooksName];
   for (let i = 0; i < hooks.length; i++) {
-    let hook = hooks[i];
-    const result = hook.tagsFunction(state.info.tags);
-    if (result) {
-      await hook.f(state);
+    let hook = hooks[i]
+    if (!hook?.tags?.length || state?.tagsMatch(hook.tags)) {
+      await hook.f(state)
     }
   }
   return state;
 };
 
-const addHook = (hooksName: string, opts: string | Hook | ((state: any) => any), f?: (state: any) => any): void => {
+const addHook = (hooksName: string, opts: string | Hook | ((state:any) => any), f?: (state:any) => any): void => {
   let hookOpts: Hook;
 
   if (isFunction(opts)) {
-    hookOpts = { name: '', f: opts, tagsFunction: () => true };
+    hookOpts = { name: '', f: opts };
   } else if (isString(opts)) {
-    hookOpts = { name: opts, f: f!, tagsFunction: () => true };
+    hookOpts = { name: opts, f: f! };
   } else if (isObject(opts)) {
-    hookOpts = opts as Hook;
-    hookOpts.f = f!;
+    hookOpts = opts as Hook
+    hookOpts.f = hookOpts.f || f!
+    hookOpts.tags = normalizeTags(hookOpts.tags)
   } else {
     throw new Error('Unknown options argument: ' + JSON.stringify(opts));
   }
 
-  hookOpts.tagsFunction = tagsFunction(hookOpts.tags);
-
   allHooks[hooksName] = concat(allHooks[hooksName], hookOpts);
 };
 
-export const BeforeAll = (opts: string | Hook | ((state: any) => any), f?: (state: any) => any): void => { addHook('beforeAll', opts, f) };
+
+export const BeforeAll = (opts: string | (() => any), f?: () => any): void => { addHook('beforeAll', opts, f) };
 export const applyBeforeAllHooks = (state: any): Promise<any> => applyHooks('beforeAll', state);
 
-export const Before = (opts: string | Hook | ((state: any) => any), f?: (state: any) => any): void => { addHook('before', opts, f) };
+export const Before = (opts: string | Hook | ((state:any) => any), f?: (state:any) => any): void => { addHook('before', opts, f) };
 export const applyBeforeHooks = (state: any): Promise<any> => applyHooks('before', state);
 
-export const BeforeStep = (opts: string | Hook | ((state: any) => any), f?: (state: any) => any): void => { addHook('beforeStep', opts, f) };
+export const BeforeStep = (opts: string | Hook | ((state:any) => any), f?: (state:any) => any): void => { addHook('beforeStep', opts, f) };
 export const applyBeforeStepHooks = (state: any): Promise<any> => applyHooks('beforeStep', state);
 
-export const AfterAll = (opts: string | Hook | ((state: any) => any), f?: (state: any) => any): void => { addHook('afterAll', opts, f) };
+export const AfterAll = (opts: string | (() => any), f?: () => any): void => { addHook('afterAll', opts, f) };
 export const applyAfterAllHooks = (state: any): Promise<any> => applyHooks('afterAll', state);
 
-export const After = (opts: string | Hook | ((state: any) => any), f?: (state: any) => any): void => { addHook('after', opts, f) };
+export const After = (opts: string | Hook | ((state:any) => any), f?: (state:any) => any): void => { addHook('after', opts, f) };
 export const applyAfterHooks = (state: any): Promise<any> => applyHooks('after', state);
 
-export const AfterStep = (opts: string | Hook | ((state: any) => any), f?: (state: any) => any): void => { addHook('afterStep', opts, f) };
+export const AfterStep = (opts: string | Hook | ((state:any) => any), f?: (state:any) => any): void => { addHook('afterStep', opts, f) };
 export const applyAfterStepHooks = (state: any): Promise<any> => applyHooks('afterStep', state);

packages/main/src/index.ts

@@ -12,10 +12,11 @@ import {
 import { explodeTags, tagsMatch, renderGherkin } from './render';
 import { DataTable } from '@cucumber/cucumber';
 import { DocString } from './models/DocString';
+import { normalizeTags } from './tags';
 
 export { setWorldConstructor, getWorldConstructor, QuickPickleWorld, QuickPickleWorldInterface } from './world';
 export { DocString, DataTable }
-export { explodeTags, tagsMatch }
+export { explodeTags, tagsMatch, normalizeTags }
 
 const featureRegex = /\.feature(?:\.md)?$/;
 
@@ -53,6 +54,13 @@ interface StepDefinitionMatch {
   parameters: any[];
 }
 
+export function formatStack(text:string, line:string) {
+  let stack = text.split('\n')
+  while(!stack[0].match(/\.feature(?:\.md)?:\d+:\d+/)) stack.shift()
+  stack[0] = stack[0].replace(/:\d+:\d+$/, `:${line}:1`)
+  return stack.join('\n')
+}
+
 export const gherkinStep = async (step: string, state: any, line: number, stepIdx:number, explodeIdx?:number, data?:any): Promise<any> => {
   const stepDefinitionMatch: StepDefinitionMatch = findStepDefinitionMatch(step);
 
@@ -71,26 +79,50 @@ export const gherkinStep = async (step: string, state: any, line: number, stepId
   }
 
   try {
-    applyBeforeStepHooks(state);
+    await applyBeforeStepHooks(state);
     try {
       await stepDefinitionMatch.stepDefinition.f(state, ...stepDefinitionMatch.parameters, data);
     }
     catch(e:any) {
-      let stack = e.stack.split('\n')
-      while(!stack[0].match('gherkinStep')) stack.shift()
-      stack.shift()
-      stack[0] = stack[0].replace(/:\d+:\d+$/, `:${state.info.line}:1`)
-      e.stack = stack.join('\n')
-      throw e
+      // Add the Cucumber info to the error message
+      e.message = `${step} (#${line})\n${e.message}`
+
+      // Sort out the stack for the Feature file
+      e.stack = formatStack(e.stack, state.info.line)
+
+      // Set the flag that this error has been added to the state
+      e.isStepError = true
+
+      // Add the error to the state
+      state.info.errors.push(e)
+
+      // If not in a soft fail mode, re-throw the error
+      if (state.isComplete || !state.tagsMatch(state.config.softFailTags)) throw e
+    }
+    finally {
+      await applyAfterStepHooks(state);
     }
-    applyAfterStepHooks(state);
   }
   catch(e:any) {
-    e.message = `${step} (#${line})\n${e.message}`
-    if (state.tagsMatch(state.config.softFailTags)) {
+
+    // If the error hasn't already been added to the state:
+    if (!e.isStepError) {
+
+      // Add the Cucumber info to the error message
+      e.message = `${step} (#${line})\n${e.message}`
+
+      // Add the error to the state
       state.info.errors.push(e)
-      if (!state.isComplete) return
     }
+
+    // If in soft fail mode and the state is not complete, don't throw the error
+    if (state.tagsMatch(state.config.softFailTags) && (!state.isComplete || !state.info.errors.length)) return
+
+    // The After hook is usually run in the rendered file, at the end of the rendered steps.
+    // But, if the tests have failed, then it should run here, since the test is halted.
+    await applyAfterHooks(state)
+
+    // Otherwise throw the error
     throw e
   }
   finally {
@@ -174,13 +206,7 @@ interface ResolvedConfig extends ViteResolvedConfig {
   quickpickle?: Partial<QuickPickleConfig>;
 }
 
-export function normalizeTags(tags?:string|string[]|undefined):string[] {
-  if (!tags) return []
-  tags = Array.isArray(tags) ? tags : tags.split(/\s*,\s*/g)
-  return tags.filter(Boolean).map(tag => tag.startsWith('@') ? tag : `@${tag}`)
-}
-
-function is3d(arr:string|string[]|string[][]):arr is string[][] {
+function is2d(arr:string|string[]|string[][]):arr is string[][] {
   return Array.isArray(arr) && arr.every(item => Array.isArray(item))
 }
 
@@ -203,7 +229,7 @@ export const quickpickle = (conf:Partial<QuickPickleConfigSetting> = {}):Plugin
       config.softFailTags = normalizeTags(config.softFailTags)
       config.concurrentTags = normalizeTags(config.concurrentTags)
       config.sequentialTags = normalizeTags(config.sequentialTags)
-      if (is3d(config.explodeTags)) config.explodeTags = config.explodeTags.map(normalizeTags)
+      if (is2d(config.explodeTags)) config.explodeTags = config.explodeTags.map(normalizeTags)
       else config.explodeTags = [normalizeTags(config.explodeTags)]
     },
     async transform(src: string, id: string) {

packages/main/src/tags.ts

@@ -1,27 +1,5 @@
-import parse from '@cucumber/tag-expressions';
-
-interface TagExpression {
-  evaluate: (tags: string[]) => boolean;
-}
-
-const parseTagsExpression = (tagsExpression: string): TagExpression => {
-  try {
-    const parsedExpression = parse(tagsExpression);
-    return parsedExpression;
-  } catch (error) {
-    throw new Error(`Failed to parse tag expression: ${(error as Error).message}`);
-  }
-}
-
-export const tagsFunction = (tagsExpression?: string): (tags: string[]) => boolean => {
-  if (!tagsExpression) {
-    return (tags: string[]) => true;
-  }
-
-  const parsedTagsExpression = parseTagsExpression(tagsExpression);
-
-  return (tags: string[]) => {
-    const result = parsedTagsExpression.evaluate(tags);
-    return result;
-  };
+export function normalizeTags(tags?:string|string[]|undefined):string[] {
+  if (!tags) return []
+  tags = Array.isArray(tags) ? tags : tags.split(/\s*,\s*/g)
+  return tags.filter(Boolean).map(tag => tag.startsWith('@') ? tag : `@${tag}`)
 }

packages/main/tests/hooks/hooks.feature

@@ -0,0 +1,34 @@
+@sequential
+Feature: Testing hooks
+
+  As a behavioral test writer
+  I need a consistent way to run code before and after steps, scenarios, and test runs
+
+  Scenario: Hooks: All hooks should work
+    Given I run the tests
+    Then the tests should pass
+
+  @soft
+  Scenario: Hooks: Hooks also work on @soft tests
+    Given I run the tests
+    Then the tests should pass
+
+  @fails
+  Scenario: Hooks: Errors are available in the hook
+    Given I run the tests
+    Then the tests should fail
+
+  @clearErrorsAfterStep @soft
+  Scenario: Hooks: The AfterStep hook can clear errors
+    Given I run the tests
+    Then the tests should fail
+
+  @clearErrorsAfterStep @fails
+  Scenario: Hooks: AfterStep must be @soft when clearing errors, or the test still fails
+    Given I run the tests
+    Then the tests should fail
+
+  @soft @fails
+  Scenario: Hooks: If the errors are not cleared, a @soft test still fails
+    Given I run the tests
+    Then the tests should fail

packages/main/tests/hooks/hooks.steps.ts

@@ -0,0 +1,69 @@
+import { expect } from 'vitest';
+import { Given, When, Then, BeforeAll, Before, BeforeStep, AfterStep, After, AfterAll, QuickPickleWorld } from 'quickpickle';
+
+const log:any = {}
+
+BeforeAll(async () => {
+  log.tests = {}
+})
+
+Before(async (state) => {
+  log.tests[state.info.scenario] = []
+  log.tests[state.info.scenario].push('Before')
+  state.hooks = {}
+  state.hooks.beforeHook = true
+})
+
+BeforeStep(async (state) => {
+  log.tests[state.info.scenario].push('BeforeStep')
+})
+
+AfterStep({ tags:'clearErrorsAfterStep' }, async (state) => {
+  state.info.errors = []
+})
+
+AfterStep(async (state) => {
+  log.tests[state.info.scenario].push('errors: ' + state.info.errors.length)
+  log.tests[state.info.scenario].push('AfterStep')
+})
+
+After(async (state) => {
+  log.tests[state.info.scenario].push('errors: ' + state.info.errors.length)
+  log.tests[state.info.scenario].push('After')
+})
+
+const testWithNoErrors = [
+  'Before',
+  'BeforeStep',
+  'errors: 0',
+  'AfterStep',
+  'BeforeStep',
+  'errors: 0',
+  'AfterStep',
+  'errors: 0',
+  'After',
+]
+
+const testWithError = [
+  'Before',
+  'BeforeStep',
+  'errors: 0',
+  'AfterStep',
+  'BeforeStep',
+  'errors: 1',
+  'AfterStep',
+  'errors: 1',
+  'After',
+]
+
+AfterAll(async () => {
+  console.log('AfterAll')
+  expect(log.tests).toMatchObject({
+    'Hooks: All hooks should work': testWithNoErrors,
+    'Hooks: Hooks also work on @soft tests': testWithNoErrors,
+    'Hooks: Errors are available in the hook': testWithError,
+    'Hooks: The AfterStep hook can clear errors': testWithNoErrors,
+    'Hooks: AfterStep must be @soft when clearing errors, or the test still fails': testWithNoErrors,
+    'Hooks: If the errors are not cleared, a @soft test still fails': testWithError,
+  })
+})

packages/main/tests/test.feature

@@ -42,8 +42,8 @@ Feature: Basic Test
       Given I run the tests
       When the tests fail
       And the tests fail
-      Then the stack for error 1 should contain "test.feature"
-      And the stack for error 2 should contain "test.feature"
+      Then the stack for error 1 should contain "test.feature:43:1"
+      And the stack for error 2 should contain "test.feature:44:1"
       And clear 2 errors
 
     @fails @soft

packages/main/tests/tests.steps.ts

@@ -1,6 +1,6 @@
 import { expect } from "vitest";
 import { Given, Then, When } from "../src";
-import type { DataTable } from "@cucumber/cucumber";
+import type { DataTable } from "../src";
 import { clone, get, set } from "lodash-es";
 import type { DocString } from "../src/models/DocString";
 
@@ -60,7 +60,7 @@ Then('the variable/value/property {string} should be {int} character(s) long', (
 })
 
 Then('the stack for error {int} should contain {string}', async (world, idx, expected) => {
-  let stack = world.info.errors[idx-1].stack
+  let stack = world.info.errors[idx-1].stack.split('\n')[0]
   await expect(stack).toContain(expected)
 })
 Then('clear error {int}', async (world, idx) => {

packages/main/vitest.workspace.ts

@@ -4,12 +4,22 @@ export default defineWorkspace([
   {
     extends: './vite.config.ts',
     test: {
+      name: 'features',
       include : [ 'tests/*.feature' ],
       setupFiles: [ 'tests/tests.steps.ts' ],
     },
   },
+  {
+    extends: './vite.config.ts',
+    test: {
+      name: 'hooks',
+      include : [ 'tests/hooks/*.feature' ],
+      setupFiles: [ 'tests/tests.steps.ts','tests/hooks/hooks.steps.ts' ],
+    },
+  },
   {
     test: {
+      name: 'unit',
       include : [ './tests/*.test.ts' ],
     }
   },