@mbarneyjr Thank you for submitting this PR! I see this is your first time submitting to DefinitelyTyped 👋 — I'm the local bot who will help you through the process of getting things through.

This is a live comment which I will keep updated.

1 package in this PR

• node — on npm, on unpkg

Code Reviews

Because you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.

You can test the changes of this PR in the Playground.

Status

• ✅ No merge conflicts
• ✅ Continuous integration tests have passed
• 🕐 Most recent commit is approved by type definition owners or DT maintainers

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Inactive

This PR has been inactive for 31 days — it is considered abandoned, and therefore closed!

{
  "type": "info",
  "now": "-",
  "pr_number": 67420,
  "author": "mbarneyjr",
  "headCommitOid": "dc1ab0060807fc2b958179cb2d59bc3d40bec6a9",
  "mergeBaseOid": "398d563b05acda2c203e5144aefa2cac9500ae33",
  "lastPushDate": "2023-11-15T19:33:43.000Z",
  "lastActivityDate": "2024-01-08T21:43:03.000Z",
  "maintainerBlessed": "Waiting for Code Reviews",
  "hasMergeConflict": false,
  "isFirstContribution": true,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": "node",
      "kind": "edit",
      "files": [
        {
          "path": "types/node/test.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/node/test/test.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "Microsoft",
        "jkomyno",
        "alvis",
        "r3nya",
        "btoueg",
        "smac89",
        "touffy",
        "DeividasBakanas",
        "eyqs",
        "Hannes-Magnusson-CK",
        "hoo29",
        "kjin",
        "ajafff",
        "islishude",
        "mwiktorczyk",
        "mohsen1",
        "n-e",
        "galkin",
        "parambirs",
        "eps1lon",
        "ThomasdenH",
        "WilcoBakker",
        "wwwy3y3",
        "samuela",
        "kuehlein",
        "bhongy",
        "chyzwar",
        "trivikr",
        "yoursunny",
        "qwelias",
        "ExE-Boss",
        "peterblazejewicz",
        "addaleax",
        "victorperin",
        "ZYSzys",
        "NodeJS",
        "LinusU",
        "wafuwafu13",
        "mcollina",
        "Semigradsky"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    }
  ],
  "reviews": [
    {
      "type": "changereq",
      "reviewer": "sheetalkamat",
      "date": "2024-01-08T21:43:03.000Z"
    },
    {
      "type": "stale",
      "reviewer": "Semigradsky",
      "date": "2023-11-16T10:57:41.000Z",
      "abbrOid": "ca5bc39"
    }
  ],
  "mainBotCommentID": 1813142149,
  "ciResult": "pass"
}

🔔 @microsoft @jkomyno @alvis @r3nya @btoueg @smac89 @Touffy @DeividasBakanas @eyqs @Hannes-Magnusson-CK @hoo29 @kjin @ajafff @islishude @mwiktorczyk @mohsen1 @n-e @galkin @parambirs @eps1lon @ThomasdenH @WilcoBakker @wwwy3y3 @samuela @kuehlein @bhongy @chyzwar @trivikr @yoursunny @qwelias @ExE-Boss @peterblazejewicz @addaleax @victorperin @ZYSzys @nodejs @LinusU @wafuwafu13 @mcollina @Semigradsky — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

@mbarneyjr One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

I think it makes sense to use a function with the same signature though, unless there's edge cases I'm not considering/misunderstanding something

When using sinon, the @types/sinon package defines types that use the stubbed/mocked function for arguments to callsFake(), as well as the resolves(), returns(), etc methods, and I've found this very helpful when mocking things throughout my tests. Because I'm forced to return a correct type from my mocks, none of my code breaks due to type errors. This PR is to update the built-in test module's types up to the same level

Is there anything specific you'd like me to change?

Hello! When I added these types I want to make passable all tests from Node.js, some tests mock functions with another signature.

But maybe require same signature make sense, different signature is 'corner case' that can be handled by type assertion. Lets wait what think other contributors.

In any case make sense to change not only mockImplementation*. The functions fn, method, getter, setter also have Implementation type which can be changed to specific F or MockedObject[MethodName].

Gotcha, thank you! I believe this already worked for fn, but it should now also work for the various ways of calling method (with/without implementation, with { getter: true }/{ setter: true }, etc), as well as the getter and setter functions. It seems to work as expected, but if there's anything missing/incorrect I'm happy to fix. Thanks for considering this PR!

@Semigradsky Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

Re-ping @microsoft, @jkomyno, @alvis, @r3nya, @btoueg, @smac89, @Touffy, @DeividasBakanas, @eyqs, @Hannes-Magnusson-CK, @hoo29, @kjin, @ajafff, @islishude, @mwiktorczyk, @mohsen1, @n-e, @galkin, @parambirs, @eps1lon, @ThomasdenH, @WilcoBakker, @wwwy3y3, @samuela, @kuehlein, @bhongy, @chyzwar, @trivikr, @yoursunny, @qwelias, @ExE-Boss, @peterblazejewicz, @addaleax, @victorperin, @ZYSzys, @nodejs, @LinusU, @wafuwafu13, @mcollina, @Semigradsky:

This PR has been out for over a week, yet I haven't seen any reviews.

Could someone please give it some attention? Thanks!

It has been more than two weeks and this PR still has no reviews.

I'll bump it to the DT maintainer queue. Thank you for your patience, @mbarneyjr.

(Ping @microsoft, @jkomyno, @alvis, @r3nya, @btoueg, @smac89, @Touffy, @DeividasBakanas, @eyqs, @Hannes-Magnusson-CK, @hoo29, @kjin, @ajafff, @islishude, @mwiktorczyk, @mohsen1, @n-e, @galkin, @parambirs, @eps1lon, @ThomasdenH, @WilcoBakker, @wwwy3y3, @samuela, @kuehlein, @bhongy, @chyzwar, @trivikr, @yoursunny, @qwelias, @ExE-Boss, @peterblazejewicz, @addaleax, @victorperin, @ZYSzys, @nodejs, @LinusU, @wafuwafu13, @mcollina, @Semigradsky.)

@mbarneyjr One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

@mbarneyjr I haven't seen any activity on this PR in more than three weeks, and it still has problems that prevent it from being merged. The PR will be closed on Feb 7th (in a week) if the issues aren't addressed.

@mbarneyjr To keep things tidy, we have to close PRs that aren't mergeable and don't have activity in the last month. No worries, though — please open a new PR if you'd like to continue with this change. Thank you!