feat: add type coverage #1102

saimeunt · 2024-04-26T11:25:20Z

Motivation and Resolution

This PR adds a new ts:coverage npm script that uses typescript-coverage-report to generage a TypeScript coverage report available in the coverage-ts directory.

On initial run, every files had >80% coverage except 4:
src/utils/assert.ts
src/utils/calldata/formatter.ts
src/utils/calldata/parser/parser-2.0.0.ts
src/utils.responseParser/index.ts

I added some additional typings on these files to reach >80% coverage (not sure if these modifications are 100% correct though).

Resolves: #1088

RPC version (if applicable)

0.7.1

Usage related changes

None

Development related changes

New npm script added: ts:coverage

Checklist:

Performed a self-review of the code
Rebased to the last commit of the target branch (or merged it into my branch)
Linked the issues which this PR resolves
Documented the changes in code (API docs will be generated automatically)
Updated the tests
All tests are passing

* chore(release): 6.8.0 [skip ci] # [6.8.0](starknet-io/starknet.js@v6.7.0...v6.8.0) (2024-04-23) ### Bug Fixes * starkne types 0.7 ([starknet-io#1087](starknet-io#1087)) ([b038c76](starknet-io@b038c76)) * tslib ([starknet-io#1068](starknet-io#1068)) ([dd7dc10](starknet-io@dd7dc10)) * **utils:** fix block identifier ([starknet-io#1076](starknet-io#1076)) ([0a3499d](starknet-io@0a3499d)) ### Features * add getGasPrice rpc provider method ([starknet-io#1056](starknet-io#1056)) ([d396275](starknet-io@d396275)) * Export function parseCalldataField() ([4d59658](starknet-io@4d59658)) * rpc 0.7.1 ([starknet-io#1071](starknet-io#1071)) ([11dc600](starknet-io@11dc600)) * docs: add paragrapher for encode decode tool * Update CHANGELOG.md --------- Co-authored-by: Toni Tabak <tabaktoni@gmail.com> Co-authored-by: semantic-release-bot <semantic-release-bot@martynus.net> Co-authored-by: Ivan Pavičić <ivan.pavicic@live.com>

@return

* chore: add examples to JsDoc for num.ts file * chores: implement requested update changes examples to JsDoc for num.ts file * chore: change @return to @returns for JsDoc examples in num.ts file --------- Co-authored-by: Toni Tabak <tabaktoni@gmail.com> Co-authored-by: semantic-release-bot <semantic-release-bot@martynus.net>

ivpavici

thanks!
we were thinking about removing this whole coverage-ts folder , and instead just provide the option for developers to locally check if the type coverage is ok in the console... and this could be also added to our CI on every PR. So thinking about
https://github.com/plantain-00/type-coverage instead of https://github.com/alexcanessa/typescript-coverage-report

saimeunt · 2024-04-27T13:45:40Z

@ivpavici I removed the coverage-ts folder and switched to type-coverage as requested.
The ts:coverage command will now display a warning eg. The type coverage rate is lower than the target(95%).
The 95% target is arbitrary idk if we should target higher or lower.
Also I couldn't find a way with this package to display precisely the specific files having a low type coverage contrary to typescript-coverage-report.

…s_to_domain refactor: getStarkName & getStarkProfile to work with latest version of contract

ivpavici · 2024-04-29T09:53:55Z

thanks!
Maybe we could also add a note for future devs about running this step manually here:
https://github.com/starknet-io/starknet.js/blob/develop/CONTRIBUTING.md#other-notes

…o#1107)

…-io#1106)

…t-io#1105)" (starknet-io#1108) This reverts commit 0ab0de7.

PhilippeR26 · 2024-04-29T14:07:44Z

src/utils/responseParser/index.ts

@@ -11,13 +13,13 @@ import {
 import type { GetTransactionReceiptResponse } from '../transactionReceipt';

 export abstract class ResponseParser {
-  abstract parseGetBlockResponse(res: any): GetBlockResponse;
+  abstract parseGetBlockResponse(res: BlockWithTxHashes): GetBlockResponse;


I have a serious doubt for this one.
@tabaktoni should have a look at all the types you added.

PhilippeR26

The global ts coverage percentage is nice to have, but if the result is bad, the dev has nothing to identify what and where are the problems.
The report is anyway useful if the percentage has to be improved.
Maybe a npm command just to get the result value, but also a command to have the report if requested by the dev. Will have to be explained (also how to display the report) in CONTRIBUTE doc.

saimeunt · 2024-05-01T23:50:58Z

@PhilippeR26 Thanks for the review, I added typescript-coverage-report back and ignored the default output coverage-ts in .gitignore.
@ivpavici I've added a small note about the 2 new npm scripts introduced in CONTRIBUTING.md

PhilippeR26

After analysis, I think that the types added are good, but has to be confirmed by @tabaktoni .
Script for coverage report is generating a full screen warning :

Warning: Table: Support for defaultProps will be removed from function components in a future major release. Use JavaScript default parameters instead.
    at Table (/home/edmond/Documents/starknet/starknet.js/node_modules/semantic-ui-react/dist/commonjs/collections/Table/Table.js:40:24)
    at div
    at Container 
...

Can you solve that?

You should add in Contribute (otherwise sure it will be asked in the assistance ...) something like : launch ./coverage-ts/index.html in your browser.
Target your PR to next-version

* chore(release): 6.8.0 [skip ci] # [6.8.0](starknet-io/starknet.js@v6.7.0...v6.8.0) (2024-04-23) ### Bug Fixes * starkne types 0.7 ([starknet-io#1087](starknet-io#1087)) ([b038c76](starknet-io@b038c76)) * tslib ([starknet-io#1068](starknet-io#1068)) ([dd7dc10](starknet-io@dd7dc10)) * **utils:** fix block identifier ([starknet-io#1076](starknet-io#1076)) ([0a3499d](starknet-io@0a3499d)) ### Features * add getGasPrice rpc provider method ([starknet-io#1056](starknet-io#1056)) ([d396275](starknet-io@d396275)) * Export function parseCalldataField() ([4d59658](starknet-io@4d59658)) * rpc 0.7.1 ([starknet-io#1071](starknet-io#1071)) ([11dc600](starknet-io@11dc600)) * chore/add JsDoc for address.ts file * update/ change address jsDoc - Add quotes to address string - Change address to get more zeros * feat/ Add JSDoc for encode.ts * update/ fix comments of encode.ts * update/ Fix last comment --------- Co-authored-by: Toni Tabak <tabaktoni@gmail.com> Co-authored-by: semantic-release-bot <semantic-release-bot@martynus.net>

tabaktoni · 2024-05-03T10:34:08Z

In general looking lgtm

ivpavici · 2024-05-03T11:24:57Z

@saimeunt can you please fix the last comments from Phil? and we can merge 👍

saimeunt · 2024-05-03T12:40:30Z

Will do tonight!

…eunt/starknet.js into feat/add-type-coverage-1088

saimeunt · 2024-05-04T01:50:07Z

@PhilippeR26 I was unable to make the warning disappear, it's caused by a non-critical issue in semantic-ui-react which is a dependency of typescript-coverage-report, this dependency will have to be updated to get rid of the issue.

@ivpavici I'm closing this PR in favor of #1120 which is properly targetting next-version.

irisdv and others added 4 commits April 22, 2024 16:02

refactor: getStarkName & getStarkProfile for latest version of contract

d35f154

feat: add type coverage

d0fdd73

ivpavici reviewed Apr 26, 2024

View reviewed changes

feat: switch to type-coverage

d56cad2

Merge pull request starknet-io#1093 from irisdv/ref/starknetid_addres…

8ba8d10

…s_to_domain refactor: getStarkName & getStarkProfile to work with latest version of contract

ivpavici requested review from tabaktoni and PhilippeR26 April 29, 2024 09:51

NueloSE and others added 4 commits April 29, 2024 12:25

chore: Add JsDoc comments and examples for merkle.ts file (starknet-i…

f77bae5

…o#1107)

chore: Add JsDoc comments and examples for selector.ts file (starknet…

0b25aba

…-io#1106)

chore: add examples to JsDoc for transaction.ts file (starknet-io#1105)

0ab0de7

Revert "chore: add examples to JsDoc for transaction.ts file (starkne…

59eb01e

…t-io#1105)" (starknet-io#1108) This reverts commit 0ab0de7.

PhilippeR26 reviewed Apr 29, 2024

View reviewed changes

PhilippeR26 requested changes Apr 29, 2024

View reviewed changes

saimeunt added 2 commits May 2, 2024 01:38

fix: add back typescript-coverage-report

d9d2968

fix: document how to display the type coverage report

00342f2

PhilippeR26 requested changes May 2, 2024

View reviewed changes

tabaktoni added the Type: feature New feature or request label May 3, 2024

saimeunt added 4 commits May 4, 2024 03:07

feat: add type coverage

fd471d3

feat: switch to type-coverage

35e3c8b

fix: add back typescript-coverage-report

1dd68b4

fix: document how to display the type coverage report

73e63ed

saimeunt added 2 commits May 4, 2024 03:07

fix: clarify how to view typespcript-coverage-report HTML report

78ff4f6

Merge branch 'feat/add-type-coverage-1088' of https://github.com/saim…

705ecdd

…eunt/starknet.js into feat/add-type-coverage-1088

saimeunt closed this May 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add type coverage #1102

feat: add type coverage #1102

saimeunt commented Apr 26, 2024

ivpavici left a comment

saimeunt commented Apr 27, 2024

ivpavici commented Apr 29, 2024 •

edited

Loading

PhilippeR26 Apr 29, 2024

PhilippeR26 left a comment •

edited

Loading

saimeunt commented May 1, 2024

PhilippeR26 left a comment

tabaktoni commented May 3, 2024

ivpavici commented May 3, 2024

saimeunt commented May 3, 2024

saimeunt commented May 4, 2024

feat: add type coverage #1102

feat: add type coverage #1102

Conversation

saimeunt commented Apr 26, 2024

Motivation and Resolution

RPC version (if applicable)

Usage related changes

Development related changes

Checklist:

ivpavici left a comment

Choose a reason for hiding this comment

saimeunt commented Apr 27, 2024

ivpavici commented Apr 29, 2024 • edited Loading

PhilippeR26 Apr 29, 2024

Choose a reason for hiding this comment

PhilippeR26 left a comment • edited Loading

Choose a reason for hiding this comment

saimeunt commented May 1, 2024

PhilippeR26 left a comment

Choose a reason for hiding this comment

tabaktoni commented May 3, 2024

ivpavici commented May 3, 2024

saimeunt commented May 3, 2024

saimeunt commented May 4, 2024

ivpavici commented Apr 29, 2024 •

edited

Loading

PhilippeR26 left a comment •

edited

Loading