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

Verify early that test files actually define the corresponding named modules #1867

Closed
mernst-github opened this issue May 5, 2020 · 6 comments
Labels
bug cleanup Tech debt, resolving it improves our own velocity

Comments

@mernst-github
Copy link

#1308 # Relevant Rules

karma_web_test

Description

The scripts generated by karma_web_test loadFile()s all the files and then require()s them as modules:

...
 loadFile(
      "workspace/src/test/ts/..._test.js",
      "var __awaiter = ...");
  loadFile(
      "../../../../../../../../_tmp/38e45c1d5edcea94c71b05fa49e4545d/tmp-18895cP3fwIlj2RG3.js",
      "\n      // A simplified version of Karma's requirejs.config.tpl.js f...)

The unstated assumption is that ..._test.js actually defines a named module '..._test' . If that assumption doesn't hold the errors are really hard to grasp for a newbie, namely
'no timestamp' warnings, 404s in the web server (since the module name doesn't even map to the right url) and a 30s timeout (since we never reach karma.start).

I did not find this documented anywhere and the mechanism in the examples/ is also fairly obscure:

// at least one import or export is needed for this file to
// be compiled into an named-UMD module by typescript
export {}; 

To reproduce, try to use an empty .js file as "src" of a karma_web_test rule.

Describe the solution you'd like

The requirements for the contents of test files should be documented more clearly.
Also, after/as part of each loadFile, karma_web_test should check that the file indeed defined the right named module and abort early with a useful error message.

Describe alternatives you've considered

Maybe even a build-time check on the test files whether they're named modules?

@mernst-github
Copy link
Author

#1872 is a "wonderful" example of the confusion that arises when people try to debug these issues. A better error would really be appreciated!

@applmak
Copy link
Contributor

applmak commented May 17, 2020

+1 This caused me much wasted time.

@mernst-github
Copy link
Author

Alternatively, instead of assuming that test files create certain modules, one could use this (semi-public) requirejs api to learn which modules are defined and issue the respective 'require' calls.
https://github.com/requirejs/requirejs/wiki/Internal-API:-onResourceLoad
(I just read up on this, I have no experience whether the api works as expected.)

@github-actions
Copy link

github-actions bot commented Sep 8, 2020

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Sep 8, 2020
@alexeagle alexeagle added the cleanup Tech debt, resolving it improves our own velocity label Sep 8, 2020
@github-actions github-actions bot removed the Can Close? We will close this in 30 days if there is no further activity label Jan 23, 2021
@papshed
Copy link

papshed commented Nov 24, 2021

+1 This caused us much wasted time.

@alexeagle
Copy link
Collaborator

Sorry everyone - since karma_web_test is no longer in this repo, I'm closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cleanup Tech debt, resolving it improves our own velocity
Projects
None yet
Development

No branches or pull requests

5 participants