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

No implicit test controller access from node_modules directory #8121

Closed
ChrisHamilton-kxs opened this issue Jan 17, 2024 · 11 comments
Closed
Labels
TYPE: bug The described behavior is considered as wrong (bug).

Comments

@ChrisHamilton-kxs
Copy link

ChrisHamilton-kxs commented Jan 17, 2024

What is your Scenario?

I'm working on exporting a testing library that wraps testcafe and provides page classes specific to my company's app. The library makes liberal use of implicit test controller access, even in async methods. I know the official docs say implicit access is not supported in async context, but it works fine when executed locally, hence why people have been using this pattern. However, it breaks when importing the library into another project, with errors indicating testcafe does not have implicit access to the test controller.

In fact, when the code is just copy/pasted into the node_modules directory, it stops working. Furthermore, it stops working only after the second attempt to access the test controller. The first attempt works fine.

I've created a minimal example that just attempts to click on the body of google.com twice. The fixture contains two tests that use the exact same async function imported from different places.

I've confirmed that passing the test controller as a parameter fixes the issue, but this will require extensive refactoring across an enormous library.

An explanation of why this works at top level but not in node_modules would be great.

What is the Current behavior?

 Running tests in:
 - Chrome 120.0.0.0 / Windows 10

 Testcafe bug repro
begin
clicked once
clicked twice
 √ succeeds
clicked once
 × fails

   1) The action does not have implicit test controller access. Reference the 't' object to gain it.

      Browser: Chrome 120.0.0.0 / Windows 10

          3 |    console.log("begin");
          4 |
          5 |    await t.click("body");
          6 |    console.log("clicked once");
          7 |
       >  8 |    await t.click("body");
          9 |    console.log("clicked twice");
         10 |}
         11 |exports.clickBodyTwice = clickBodyTwice;
         12 |

         at clickBodyTwice (C:\repos\testcafe-bug\node_modules\@@bug-repro\click-body-twice.js:8:10)



 1/2 failed (5s)

What is the Expected behavior?

Both tests pass, since they are executing the exact same code.

What is the public URL of the test page? (attach your complete example)

www.google.com

What is your TestCafe test code?

testcafe-bug.zip

test.js

import { clickBodyTwice as topLevelFn } from "./@@bug-repro/click-body-twice";
import { clickBodyTwice as nodeModuleFn } from "./node_modules/@@bug-repro/click-body-twice";

fixture("Testcafe bug repro")

test("succeeds", async (t) => {
    await t.navigateTo("http://www.google.com");
    await topLevelFn();
})

test("fails", async (t) => {
    await t.navigateTo("http://www.google.com");
    await nodeModuleFn();
})

click-body-twice.js

const { t } = require("testcafe");
async function clickBodyTwice() {
	console.log("begin");

	await t.click("body");
	console.log("clicked once");

	await t.click("body");
	console.log("clicked twice");
}
exports.clickBodyTwice = clickBodyTwice;

Your complete configuration file

No response

Your complete test report

No response

Screenshots

No response

Steps to Reproduce

  1. Unzip the provided file
  2. Run npm i
  3. Copy @@bug-repro into node_modules
  4. Run npx testcafe chrome .\test.js

TestCafe version

3.5.0

Node.js version

18.19.0
20.11.0

Command-line arguments

testcafe chrome .\test.js

Browser name(s) and version(s)

No response

Platform(s) and version(s)

No response

Other

No response

@ChrisHamilton-kxs ChrisHamilton-kxs added the TYPE: bug The described behavior is considered as wrong (bug). label Jan 17, 2024
@need-response-app need-response-app bot added the STATE: Need response An issue that requires a response or attention from the team. label Jan 17, 2024
@ChrisHamilton-kxs
Copy link
Author

I've also asked a stackoverflow question looking for the reason why this happens: https://stackoverflow.com/questions/77841249/why-does-this-async-function-work-when-imported-from-source-directory-but-not-wh

@ChrisHamilton-kxs
Copy link
Author

I can confirm that targetting es2015 through the Typescript compiler fixes the issue by removing async / await from the code. However, this makes me worried that code in node_modules gets treated differently than in the source directory. Can I expect any other problems by exporting this library?

@ChrisHamilton-kxs
Copy link
Author

Some debugging info:

I see that you're using the stacktrace to retrieve the test controller ID. This mechanism seems to succeed in my source directory, but fails in node_modules. This makes me think that you're doing some transpilation, which probably has something like a glob filter to exclude node_modules.

Here are the different stack traces when attempting to get the controller ID for the second time. The first t.click yields the same stack trace for both, which is identical to the first image. The only malformed stacktrace occurs during the second t.click in node_modules.

From my source directory
image

From node_modules (no controller ID)
image

@PavelMor25
Copy link
Collaborator

Hello @ChrisHamilton-kxs,

Everything is fine. We used the callsite package to obtain V8's CallSites and get the test controller ID. However, the research will take some time. Currently, our resources are directed towards other tasks. We recommend using import t only from test code located in your project, but not in node_modules.

@need-response-app need-response-app bot removed the STATE: Need response An issue that requires a response or attention from the team. label Jan 23, 2024
@ChrisHamilton-kxs
Copy link
Author

ChrisHamilton-kxs commented Jan 23, 2024

@PavelMor25 Why did you close this as completed? If you don't plan on fixing this should you not mark it as "Won't do"?

@need-response-app need-response-app bot added the STATE: Need response An issue that requires a response or attention from the team. label Jan 23, 2024
@PavelMor25
Copy link
Collaborator

Hello @ChrisHamilton-kxs, my apologies. I closed it with another status. However, if you still need this functionality, you can submit a pull request with a description. We will review it and accept it if everything is okay.

@PavelMor25 PavelMor25 removed the STATE: Need response An issue that requires a response or attention from the team. label Jan 25, 2024
@ChrisHamilton-kxs
Copy link
Author

ChrisHamilton-kxs commented Jan 25, 2024

@PavelMor25 why would anyone submit a PR for a closed ticket? If you agree this is a bug and would accept a fix then please reopen the ticket.

@need-response-app need-response-app bot added the STATE: Need response An issue that requires a response or attention from the team. label Jan 25, 2024
@PavelMor25
Copy link
Collaborator

Hello,

This is the first instance of using TestCafe in this usage scenario that I am aware of. We discussed the issue with the team and concluded that it is not a bug, so we will not reopen it.

@PavelMor25 PavelMor25 removed the STATE: Need response An issue that requires a response or attention from the team. label Jan 30, 2024
@ChrisHamilton-kxs
Copy link
Author

Hello,

This is the first instance of using TestCafe in this usage scenario that I am aware of. We discussed the issue with the team and concluded that it is not a bug, so we will not reopen it.

Only one user reported the issue so it isn't a bug?
And you will not support exporting a library with TestCafe as a dependency?
That seems like a standard expectation of any node_module, that you can list it as a dependency in your package.json.

If you're not going to admit this is a bug, then that means you will not accept a fix?

That's one way to do open source I guess...

@need-response-app need-response-app bot added the STATE: Need response An issue that requires a response or attention from the team. label Jan 30, 2024
@aleks-pro
Copy link
Contributor

Hello @ChrisHamilton-kxs ,

Sorry for the late response.

Only one user reported the issue so it isn't a bug?

Please note that TestCafe is an E2E testing tool out of the box. So, we do not consider it a regular npm package whose purpose is to reuse implemented logic. That is why we did not implement the functionality you require at the very beginning and consider it a missing feature.

If you're not going to admit this is a bug, then that means you will not accept a fix?

If you create a PR that addresses the issue, we will review and accept it.

@need-response-app need-response-app bot removed the STATE: Need response An issue that requires a response or attention from the team. label Feb 28, 2024
@ChrisHamilton-kxs
Copy link
Author

@aleks-pro That's fine if you consider it a feature request rather than a bug.

IMO this behaviour is expected of any dependency in any language / package manager. It's assumed that if you import a dependency, use it, then export your project, that project will be able to be imported as a dependency as well. I would not consider a basic assumption like that to be a feature. Rather, this is a bug introduced by transpilation of async / await. Transpilation which seems to only be necessary due to a questionable use of the V8 stack trace API. But that's just my opinion.

But anyway, if this is a feature request, then the ticket should be classified as such, rather than closed as complete.

When you close it you're telling the community you do not agree that this should be addressed, and anyone else experiencing this will not be aware that it is a known issue. I'm a bit worried how many other issues get swept under the rug like this.

I would love to fix this, but as you can probably relate it's hard to find the time. I would appreciate if you reopen this as a feature request so the community can at least be aware of it.

This is very useful if you have an app that supports plugins. You can ship a test library for your app to authors of the plugins. This is the use case at my company.

The workaround right now is to transpile the library to es2016 before shipping, but it makes the code harder to read and debug.

@need-response-app need-response-app bot added the STATE: Need response An issue that requires a response or attention from the team. label Feb 28, 2024
@aleks-pro aleks-pro removed the STATE: Need response An issue that requires a response or attention from the team. label Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TYPE: bug The described behavior is considered as wrong (bug).
Projects
None yet
Development

No branches or pull requests

3 participants