Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: re-write some files to typescript - part 3 #489

Merged
merged 25 commits into from
Aug 23, 2023

Conversation

shadowusr
Copy link
Member

What's done?

  • re-written a few files to typescript as a preparation step for playwright support — majority of files related to server-side static html report generation

Why?

  • to introduce strong types and be able to write new code in TS

⚠️ Notes

  • every commit is atomic, meaning all checks pass on every commit
  • might be a good idea to review each commit separately: in the final diff some moved files appear as completely removed/added, but when viewing commits separately you can see what's been truly changed

@@ -2,6 +2,7 @@
/* eslint-env browser */

const {isEmpty} = require('lodash');
/** @type Record<string, (...args: unknown[]) => unknown> */
Copy link
Member Author

Choose a reason for hiding this comment

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

Пришлось добавить временно jsdoc, чтобы не ругался компилятор tsc

addSkipped(result) {
const formattedResult = this.format(result);
addSkipped(result: TestResult | TestAdapter): TestAdapter {
const formattedResult = this.format(result, SKIPPED);
Copy link
Member Author

Choose a reason for hiding this comment

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

Тут показалось логичным всегда передавать статус, но обсуждаемо.

Конкретно в этом месте есть разница в поведении в runtime. Ранее передавался undefined, следовательно условие в test-adapter:

if (utils.shouldUpdateAttempt(status)) { // ![SKIPPED, UPDATED, RUNNING, IDLE].includes(status);
    testsAttempts.set(this._testId, _.isUndefined(testsAttempts.get(this._testId)) ? 0 : testsAttempts.get(this._testId) as number + 1);
}

Выполнялось. Стали передавать SKIPPED — оно выполняться перестанет.

@@ -91,6 +91,8 @@ export class TestAdapter {
this._attempt = testsAttempts.get(this._testId) || 0;
}

image?: boolean;
Copy link
Member Author

Choose a reason for hiding this comment

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

Пришлось добавить из-за строки в static report builder:

formattedResult.image = hasImage(formattedResult);

Copy link
Member

@DudaGod DudaGod left a comment

Choose a reason for hiding this comment

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

/ok

@shadowusr shadowusr force-pushed the users/shadowusr/HERMIONE-1003.typescript-part-3 branch from 198c456 to 6d04d3e Compare August 23, 2023 06:10
@shadowusr shadowusr merged commit 7779714 into master Aug 23, 2023
4 of 5 checks passed
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