node icon indicating copy to clipboard operation
node copied to clipboard

test: add more fixtures for strip-types

Open kevinuehara opened this issue 1 year ago • 6 comments

In this MR I'm adding more tests created in this PR, testing generics and Utility Types. This PR makes part of typescript iniciative on Node.

cc: @tniessen @ErickWendel

kevinuehara avatar Jul 29 '24 21:07 kevinuehara

Can you adjust the commit message to be prefixed with "test", and start with an active verb?

Basically change the beginning to "test: add". Preferably, "test: add more fixtures for strip-types"

avivkeller avatar Aug 09 '24 02:08 avivkeller

Commit message updated @RedYetiDev <3 Thankss!

kevinuehara avatar Aug 09 '24 14:08 kevinuehara

Hi @kevinuehara I see fixtures being added, but these fixtures are not being tested

marco-ippolito avatar Aug 13 '24 14:08 marco-ippolito

HI @marco-ippolito In fact, these fixtures are testing Typescript's own and unique functionalities (Utility Types). If it were not available there would be an error when compiling for Javascript so it can be considered a test and validation of the new ts support functionality

kevinuehara avatar Aug 13 '24 14:08 kevinuehara

HI @marco-ippolito In fact, these fixtures are testing Typescript's own and unique functionalities (Utility Types). If it were not available there would be an error when compiling for Javascript so it can be considered a test and validation of the new ts support functionality

I'm not sure I understand. Where can I see those .ts files being executed or tested? There should be a place where these .ts are run and the output is tested or checking they are executed correctly by node --experimental-strip-types. For example in the original PR the file fixtures/typescript/test-typescript.ts is tested here

marco-ippolito avatar Aug 13 '24 15:08 marco-ippolito

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 87.77%. Comparing base (1d2603b) to head (ef24e34). Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #54107      +/-   ##
==========================================
- Coverage   88.06%   87.77%   -0.29%     
==========================================
  Files         651      651              
  Lines      183386   183386              
  Branches    35800    35478     -322     
==========================================
- Hits       161504   160976     -528     
- Misses      15159    15673     +514     
- Partials     6723     6737      +14     

see 71 files with indirect coverage changes

codecov[bot] avatar Aug 13 '24 16:08 codecov[bot]

HI @marco-ippolito In fact, these fixtures are testing Typescript's own and unique functionalities (Utility Types). If it were not available there would be an error when compiling for Javascript so it can be considered a test and validation of the new ts support functionality

I'm not sure I understand. Where can I see those .ts files being executed or tested? There should be a place where these .ts are run and the output is tested or checking they are executed correctly by node --experimental-strip-types. For example in the original PR the file fixtures/typescript/test-typescript.ts is tested here

Hi @marco-ippolito! I update this PR with the tests

kevinuehara avatar Sep 11 '24 20:09 kevinuehara

@RedYetiDev @marco-ippolito @ErickWendel can review this PR? 🙏

kevinuehara avatar Sep 13 '24 11:09 kevinuehara

can you please remove unrelated formatting changes?

marco-ippolito avatar Sep 13 '24 11:09 marco-ippolito

also can you amend your first 2 commits with the right author?

marco-ippolito avatar Sep 13 '24 12:09 marco-ippolito

can you please remove unrelated formatting changes?

I needed to format and break the line due to lint 🤔

kevinuehara avatar Sep 13 '24 12:09 kevinuehara

can you please remove unrelated formatting changes?

I needed to format and break the line due to lint 🤔

if it was there it means the lint has passed in the original PR 😄

marco-ippolito avatar Sep 13 '24 12:09 marco-ippolito

@marco-ippolito I'll open a new PR with the updates.... I'm having trouble updating the author on the first two commits.

kevinuehara avatar Sep 13 '24 12:09 kevinuehara