Repository navigation
fix(gui): show actual skip status of tests after gui restart, reconcile skip status after restoring history - #820
vladislav805 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Thanks a lot for your contribution! This iteration is much cleaner and based on my testing is working correctly!
I've left a couple of comments, the most important points are these:
- whether we really need rowid introduction and if not, we could revert changes related to it
- i don't think we need
_addSyntheticTestResultmethod, cause it's basically identical to the regular addTestResult
|
|
||
| private _resolveSkippedTests(tests: TestAdapter[]): void { | ||
| const affectedTests = new Set(tests.map(test => `${test.fullName}\0${test.browserId}`)); | ||
| _.remove(this._skips, ({suite, browser}) => affectedTests.has(`${suite}\0${browser}`)); |
There was a problem hiding this comment.
side thought: why do we even have this _skips fields, which is so carefully update yet not used anywhere at all. Should probably delete it in the future, but that's a different story.
| restoreTestsState({treeState, skips, attempts}: GuiReportBuilderTestsState): void { | ||
| this._testsTree.restoreState(treeState); | ||
| this._skips = skips; | ||
| this._skips.splice(0, this._skips.length, ...skips); |
There was a problem hiding this comment.
hm, now that's niche! what's the idea behind this?
There was a problem hiding this comment.
ToolRunner retains the skips array returned by getResult(). Watch updates do not rebuild the entire _tree, so assigning a new array during rollback would leave runner.tree.skips pointing to the old, modified array. splice preserves that reference. The failed watch-update test covers this.
| ].join(' AND '); | ||
|
|
||
| this.delete({ | ||
| where: `rowid = (SELECT rowid FROM ${DB_SUITES_TABLE_NAME} WHERE ${where} ORDER BY rowid DESC LIMIT 1)` |
There was a problem hiding this comment.
Same question about rowid here. Is this truly needed?
There was a problem hiding this comment.
Deleting by test, browser, status and timestamp would remove both skips from [skipped(T), success(T), skipped(T)], rather than just the trailing one. rowid lets us delete exactly one record without changing the schema. A regression test checks that the earlier attempt survives a GUI restart.
23c07ad to
f1f199a
Compare
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f1f199a to
25e5617
Compare
Continuation of #819. Original author's commit is preserved.
Changes