Emit "VariableEmitted" on adding new variable in a test step #140
Conversation
rihter007
force-pushed
the
feature/emit_variable_events
branch
from
2e75573 to
488f887
Compare
Codecov Report
@@ Coverage Diff @@
## main #140 +/- ##
==========================================
+ Coverage 63.38% 63.41% +0.03%
==========================================
Files 164 164
Lines 10284 10304 +20
==========================================
+ Hits 6518 6534 +16
Misses 3045 3045
- Partials 721 725 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Ilya <[email protected]>
rihter007
force-pushed
the
feature/emit_variable_events
branch
from
488f887 to
b67c475
Compare
| } | ||
|
|
||
| stepOutputs, err := newTestStepsVariables(t.TestStepsBundles, func(tgtID string, stepLabel string, name string, value json.RawMessage) { | ||
| emitter := emitterForStep[stepLabel] |
There was a problem hiding this comment.
this callback ends up being called by multiple goroutines handling the steps/targets, right? this read should be sync'd somehow; it's not happening at the callsite in testStepsVariables.Add either (even if it's now just a readonly, there's no guarantee something might not change this map)
There was a problem hiding this comment.
That is true, but all access is read only, so should be fine
There was a problem hiding this comment.
note the thing i wrote in parens
There was a problem hiding this comment.
I agree, but currently the map and its usage is located nearby in terms of LoCs, so I would not overcomplicate for possible future safety
|
Let's fix #84 first |
No description provided.