Skip to content

Commit 75bf99e

Browse files
authored
fix: logging of steps in verbose and debug mode (#5312)
* Initial plan * Fix: Emit step events for MetaSteps without child steps * docs: Add documentation for step logging fix --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent b249311 commit 75bf99e

File tree

3 files changed

+178
-1
lines changed

3 files changed

+178
-1
lines changed

STEP_LOGGING_FIX.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# Step Logging Fix Documentation
2+
3+
## Problem Statement
4+
Steps were not printed in logs when running either `--verbose` or `--debug` when the I actor method is called not directly in Scenario test file (e.g., in custom steps or page objects) and the custom step doesn't call any child I steps.
5+
6+
## Root Cause
7+
The `MetaStep.run()` method in `lib/step/meta.js` never emitted `event.step.started` and `event.step.finished` events for itself. It only registered a listener to attach itself to child steps. If no child steps were called, the MetaStep was never printed in verbose/debug output.
8+
9+
## Solution
10+
Modified `lib/step/meta.js` to track whether child steps are registered and conditionally emit events only if no children are registered. This prevents duplicate printing while ensuring standalone MetaSteps are visible in output.
11+
12+
## Files Changed
13+
1. `lib/step/meta.js` - Core logic fix
14+
2. `test/unit/step/meta_step_logging_test.js` - Comprehensive unit tests
15+
16+
## Testing
17+
- ✅ 6 new unit tests (all passing)
18+
- ✅ Manual testing with custom steps (verified fix works)
19+
- ✅ Existing tests (no regression)
20+
- ✅ Security scan (CodeQL: 0 alerts)
21+
- ✅ Linting (ESLint: 0 errors)
22+
23+
## Example Output
24+
25+
### Before Fix
26+
```
27+
test with custom step WITHOUT I calls
28+
Scenario()
29+
This is just a console.log
30+
✔ OK in 5ms
31+
```
32+
❌ Step name "I customStepWithoutI" is NOT printed
33+
34+
### After Fix
35+
```
36+
test with custom step WITHOUT I calls
37+
Scenario()
38+
This is just a console.log
39+
I custom step without i
40+
✔ OK in 6ms
41+
```
42+
✅ Step name is now visible!
43+
44+
## Known Limitation
45+
For synchronous MetaSteps without child steps, the step name appears after the function executes (because events are emitted in the finally block). This is an acceptable trade-off to avoid the complexity of predicting whether child steps will be registered.

lib/step/meta.js

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,24 @@ class MetaStep extends Step {
5858
this.status = 'queued'
5959
this.setArguments(Array.from(arguments).slice(1))
6060
let result
61+
let hasChildSteps = false
6162

6263
const registerStep = step => {
6364
this.setMetaStep(null)
6465
step.setMetaStep(this)
66+
hasChildSteps = true
6567
}
6668
event.dispatcher.prependListener(event.step.before, registerStep)
69+
70+
// Start timing
71+
this.startTime = Date.now()
72+
6773
// Handle async and sync methods.
6874
if (fn.constructor.name === 'AsyncFunction') {
6975
result = fn
7076
.apply(this.context, this.args)
7177
.then(result => {
78+
this.setStatus('success')
7279
return result
7380
})
7481
.catch(error => {
@@ -78,17 +85,27 @@ class MetaStep extends Step {
7885
.finally(() => {
7986
this.endTime = Date.now()
8087
event.dispatcher.removeListener(event.step.before, registerStep)
88+
// Only emit events if no child steps were registered
89+
if (!hasChildSteps) {
90+
event.emit(event.step.started, this)
91+
event.emit(event.step.finished, this)
92+
}
8193
})
8294
} else {
8395
try {
84-
this.startTime = Date.now()
8596
result = fn.apply(this.context, this.args)
97+
this.setStatus('success')
8698
} catch (error) {
8799
this.setStatus('failed')
88100
throw error
89101
} finally {
90102
this.endTime = Date.now()
91103
event.dispatcher.removeListener(event.step.before, registerStep)
104+
// Only emit events if no child steps were registered
105+
if (!hasChildSteps) {
106+
event.emit(event.step.started, this)
107+
event.emit(event.step.finished, this)
108+
}
92109
}
93110
}
94111

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
import { expect } from 'chai'
2+
import sinon from 'sinon'
3+
import event from '../../../lib/event.js'
4+
import MetaStep from '../../../lib/step/meta.js'
5+
import Step from '../../../lib/step.js'
6+
7+
describe('MetaStep Logging', () => {
8+
let eventStub
9+
10+
beforeEach(() => {
11+
event.cleanDispatcher()
12+
eventStub = {
13+
stepStarted: sinon.spy(),
14+
stepFinished: sinon.spy(),
15+
}
16+
event.dispatcher.on(event.step.started, eventStub.stepStarted)
17+
event.dispatcher.on(event.step.finished, eventStub.stepFinished)
18+
})
19+
20+
afterEach(() => {
21+
event.cleanDispatcher()
22+
})
23+
24+
it('should emit step.started and step.finished for MetaStep without child steps', () => {
25+
const metaStep = new MetaStep('I', 'doSomething')
26+
const fn = () => {
27+
// MetaStep that doesn't call any child I steps
28+
console.log('Just a console.log')
29+
}
30+
31+
metaStep.run(fn)
32+
33+
// Should emit step.started and step.finished
34+
expect(eventStub.stepStarted.calledOnce).to.be.true
35+
expect(eventStub.stepFinished.calledOnce).to.be.true
36+
expect(eventStub.stepStarted.firstCall.args[0]).to.equal(metaStep)
37+
expect(eventStub.stepFinished.firstCall.args[0]).to.equal(metaStep)
38+
})
39+
40+
it('should NOT emit events for MetaStep WITH child steps', () => {
41+
const metaStep = new MetaStep('I', 'doSomethingWithChild')
42+
const fn = () => {
43+
// Simulate a child step being registered
44+
const childStep = new Step({ helper: 'test' }, 'childAction')
45+
event.emit(event.step.before, childStep)
46+
}
47+
48+
metaStep.run(fn)
49+
50+
// Should NOT emit step.started and step.finished for the MetaStep
51+
// because it has child steps
52+
expect(eventStub.stepStarted.called).to.be.false
53+
expect(eventStub.stepFinished.called).to.be.false
54+
})
55+
56+
it('should emit events for async MetaStep without child steps', async () => {
57+
const metaStep = new MetaStep('I', 'doSomethingAsync')
58+
const fn = async () => {
59+
// Async MetaStep that doesn't call any child I steps
60+
await Promise.resolve()
61+
console.log('Just an async operation')
62+
}
63+
64+
await metaStep.run(fn)
65+
66+
// Should emit step.started and step.finished
67+
expect(eventStub.stepStarted.calledOnce).to.be.true
68+
expect(eventStub.stepFinished.calledOnce).to.be.true
69+
expect(eventStub.stepStarted.firstCall.args[0]).to.equal(metaStep)
70+
expect(eventStub.stepFinished.firstCall.args[0]).to.equal(metaStep)
71+
})
72+
73+
it('should NOT emit events for async MetaStep WITH child steps', async () => {
74+
const metaStep = new MetaStep('I', 'doSomethingAsyncWithChild')
75+
const fn = async () => {
76+
// Simulate a child step being registered
77+
const childStep = new Step({ helper: 'test' }, 'childAction')
78+
event.emit(event.step.before, childStep)
79+
await Promise.resolve()
80+
}
81+
82+
await metaStep.run(fn)
83+
84+
// Should NOT emit step.started and step.finished for the MetaStep
85+
// because it has child steps
86+
expect(eventStub.stepStarted.called).to.be.false
87+
expect(eventStub.stepFinished.called).to.be.false
88+
})
89+
90+
it('should set status to success when MetaStep completes without error', () => {
91+
const metaStep = new MetaStep('I', 'doSomething')
92+
const fn = () => {
93+
return 'success'
94+
}
95+
96+
metaStep.run(fn)
97+
98+
expect(metaStep.status).to.equal('success')
99+
})
100+
101+
it('should set status to failed when MetaStep throws error', () => {
102+
const metaStep = new MetaStep('I', 'doSomethingThatFails')
103+
const fn = () => {
104+
throw new Error('Test error')
105+
}
106+
107+
try {
108+
metaStep.run(fn)
109+
} catch (err) {
110+
// Expected to throw
111+
}
112+
113+
expect(metaStep.status).to.equal('failed')
114+
})
115+
})

0 commit comments

Comments
 (0)