X Tutup
Skip to content

Convert E2E tests to Typescript#257

Merged
kravets-levko merged 4 commits intomainfrom
convert-e2e-to-typescript
May 14, 2024
Merged

Convert E2E tests to Typescript#257
kravets-levko merged 4 commits intomainfrom
convert-e2e-to-typescript

Conversation

@kravets-levko
Copy link
Copy Markdown
Contributor

@kravets-levko kravets-levko commented Apr 29, 2024

Part of PECO-1390

Signed-off-by: Levko Kravets <levko.ne@gmail.com>
Signed-off-by: Levko Kravets <levko.ne@gmail.com>
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.06%. Comparing base (fb817b5) to head (3806204).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #257   +/-   ##
=======================================
  Coverage   93.06%   93.06%           
=======================================
  Files          65       65           
  Lines        1586     1586           
  Branches      280      280           
=======================================
  Hits         1476     1476           
  Misses         46       46           
  Partials       64       64           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -1,22 +0,0 @@
const fs = require('fs');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We do not have ts counterpart for this?

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.

I removed it because we don't really need custom logging in tests

Signed-off-by: Levko Kravets <levko.ne@gmail.com>

const result = await resultHandler.fetchNext({ limit: rowsCount });

// @ts-expect-error TS2339: Property _data does not exist on type IOperation
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you planning any additional PRs to resolve these type incompatibilities? I assume these comments are like compiler directives saying that we know this property is not part of the type def?

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.

Yes, you're totally correct. Fields marked as private in TS become a regular public fields in JS. And in tests we used this quite a lot to inspect internal state of objects. When convering to TS, I fixed what was easy to fix, and added these @ts-expect-error directives for non-trivial ones. After we have all the tests in TS, I'll do another iteration and hopefully clean these up

@kravets-levko kravets-levko merged commit 3eed509 into main May 14, 2024
@kravets-levko kravets-levko deleted the convert-e2e-to-typescript branch May 14, 2024 18:28
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.

4 participants

X Tutup