Skip to content

Refresh tests tree#773

Open
sonic16x wants to merge 3 commits into
masterfrom
refresh-test-tree
Open

Refresh tests tree#773
sonic16x wants to merge 3 commits into
masterfrom
refresh-test-tree

Conversation

@sonic16x

@sonic16x sonic16x commented Jun 2, 2026

Copy link
Copy Markdown
Contributor
Screenshot 2026-06-02 at 19 13 58

@pkg-pr-new

pkg-pr-new Bot commented Jun 2, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/html-reporter@773

commit: 9fe4978

@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

✅ Component tests succeed

Report

@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown

✅ E2E tests succeed

Report

@sonic16x
sonic16x force-pushed the refresh-test-tree branch 2 times, most recently from cfe0215 to 60bf6ea Compare June 4, 2026 13:52
@sonic16x
sonic16x force-pushed the refresh-test-tree branch 3 times, most recently from c135c1e to 3ad54fc Compare July 2, 2026 20:12
@sonic16x
sonic16x force-pushed the refresh-test-tree branch from 3ad54fc to 2a5956d Compare July 13, 2026 05:22
Comment thread lib/gui/tool-runner/index.ts Outdated
}

async refreshTests(): Promise<void> {
this._ensureReportBuilder().saveDb();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to save db before initializing?

Why do "refreshTests" method calls reinit?

Maybe it would be better to pull "what is necessary" from "initialize" to extra method?

Perhaps we could skip saving db this way

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need it for using merge mechanism in initialize. For example, If we run some tests and then refresh tests, first we need save current result and then reinit for read new tests and merge it with current results.
In other case we should realize merge logic second time in frontend.

Comment on lines +221 to +225
useEffect(() => {
if (currentBrowser && !currentBrowserEntity && isInitialized && visibleTreeNodeIds.length > 0) {
onSelectFirstResult();
}
}, [currentBrowser, currentBrowserEntity, isInitialized, visibleTreeNodeIds.length]);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is it for?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For select first result in case when current selected test after refresh was deleted

Comment thread lib/sqlite-client.ts Outdated
Comment on lines +123 to +125
save(): void {
fs.writeFileSync(this._dbPath, this._db.export());
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"close" is sync because we can't call async operations on sync "exit" event

But "save" is what you call from async code. Its better to have it async.

@sonic16x sonic16x Jul 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@sonic16x
sonic16x force-pushed the refresh-test-tree branch from 2a5956d to b79cc0d Compare July 16, 2026 08:35
Comment on lines +80 to +82
}).catch(() => {
logError(new Error('ololo'));
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

import {copyAndUpdate} from '../../test-result/utils';

export const handleTestResults = (testplane: TestplaneWithHtmlReporter, reportBuilder: GuiReportBuilder, client: EventSource): void => {
testplane.removeAllListeners();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It removes all listeners, including these ones:
https://nda.ya.ru/t/nu5584BV7ik3kV

You can't do that.

You have that "event listeners" problem because you call "initialize" second time.
"just removing event listeners, while keeping second init" is sweeping the problem under the carpet.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants