vbhanuchander-lang commented on issue #6357:
URL: https://github.com/apache/hop/issues/6357#issuecomment-5446533028
I traced this and it looks fixed in 2.19.0 — @dsanderbi, could you confirm
on 2.19.0 or later?
**Why the variables were lost**
The active unit test is cached on the pipeline graph's state map under
`ActiveUnitTest`, and two different code paths use it:
* `LocationMouseDoubleClickExtensionPoint` — double-clicking the input data
set flask opens the **Dataset Location** dialog (the second screenshot in your
steps), and on OK it does
`getSerializer(PipelineUnitTest.class).save(unitTest)` where `unitTest` is the
**cached** object, not one loaded from metadata.
* `HopGuiUnitTestChanged` — the extension point that runs when a unit test
is created/updated/deleted.
On 2.16 the second one was only this:
```java
if (!(object instanceof PipelineUnitTest)) {
return;
}
TestingGuiPlugin.refreshUnitTestsList();
```
It refreshed the toolbar combo and nothing else. So when you added
`BUFFER_PARAM` in the unit test editor and saved, the file on disk got the
variable but the cached object on the state map did **not** — it went stale at
that moment. The run still failed as you expected, because execution reads the
variables separately. Then editing the data set and pressing OK on the Dataset
Location dialog saved the stale cached object straight over the file, and the
variable was gone. That matches your reproduction step for step, including why
the test passes again afterwards.
**What changed**
`HopGuiUnitTestChanged` now re-applies the test's variables and puts the
refreshed object back on the state map, so the cache can no longer go stale
behind the dialog:
```java
PipelineUnitTest active = TestingGuiPlugin.getCurrentUnitTest(pipelineMeta);
if (active != null && unitTest.getName().equals(active.getName())) {
UnitTestGraphVariables.apply(pipelineGraph.getVariables(), unitTest,
pipelineGraph.getStateMap());
// Keep state map pointing at the updated metadata object
pipelineGraph.getStateMap().put(DataSetConst.STATE_KEY_ACTIVE_UNIT_TEST,
unitTest);
}
```
That came in with 44c3e5ca1e ("issue #7719 : unit test variables at
design-time and unit test UX fixes", #7872, 2026-08-11), first released in
2.19.0. `HopGuiUnitTestVariablesExtensionPoint` also gained an explicit reload
from metadata for the same reason.
Checked the handler at both tags: 2.16.0-rc1 has no write to
`STATE_KEY_ACTIVE_UNIT_TEST` anywhere in the file; 2.19.0-rc1 does.
**One thing I would still consider changing**
`LocationMouseDoubleClickExtensionPoint` still saves the cached object
rather than reloading first, so it depends entirely on every writer keeping
that cache fresh. `HopGuiUnitTestVariablesExtensionPoint` already takes the
safer route and reloads from metadata before use. Making the two save sites in
the double-click handler do the same would remove the whole class of
stale-overwrite, rather than the one path that has been closed. Happy to send
that as a small PR if maintainers think it is worth it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
