Feat: Check for **/*.test.ts files, give helpful error messaging

[SephReed]

I opened a Stack Overflow question immediately after starting to use AVA

https://stackoverflow.com/questions/59596300/what-do-i-name-my-files-how-do-i-run-a-single-test

There was ultimately no way for me to tell that AVA doesn't do Typescript by default, which is strange given it boasts of coming with typescript definitions.

After first finding the issue detailing making TS a default part, I eventually found the docs for this: https://github.com/avajs/ava/blob/master/docs/recipes/typescript.md

This whole process would have been a lot easier if AVA simply checked for *.test.ts files, and then said:

Hey, you can't use TypeScript without setting that up. Check out {{link-to-docs}}

In particular, this command...

$ ava ./src/Derivations/Sources/__tests/Source.test.ts 

... had more than enough opportunity to tell me what I was doing wrong.

[novemberborn]

@SephReed please keep in mind that this project is a hobby. It doesn't pay my bills. I don't owe you anything.

I appreciate your feedback but its phrasing could be more appreciative of this being a non-commercial open source project.

[SephReed]

I'm not the person who would need this anymore feature anymore. It wouldn't be for me, it would be for the next person.

I terms of appreciation, I'm sorry, I know the feel. That being said, I still feel that I'm the one taking a risk in trusting your project. It could be a total dud, a waste of my time. It says a lot of nice things about itself on the landing page... but what if you're just lying to me?

In any case, getting passed that hurdle, I like the project.

If it's any consolation, I'd like to improve your error messaging. I'll put in a PR later.

[novemberborn]

Sure, thanks.

I’m not sure if this is easily accomplished however. master works rather differently from AVA 2. The good news is that we can more easily add proper TypeScript support now.

[novemberborn]

(You no longer pass file paths on the CLI: they’re treated as glob patterns that filter the test files selected based on the (default) configuration. But you could specify .ts extensions in the configuration and then it wouldn’t work. Unless you configured it for our Babel integration and had that set up correctly. Or are using ts-node. Neither of which are easily determined.)

[SephReed]

You no longer pass file paths on the CLI

A glob pattern can still be an absolute path though, yeah? It would just internally not actually be a path, but still act like one, right?

---

Current thoughts on the PR I want to make

1. Create a ProjectStructure.md doc which basically says

   • These are the globs AVA looks for
   • These are some ways a project is typically structured (for total newbies)
   • If you use TypeScript check out these other docs
   • How to run a simple test (project wide, folder wide, single file)
   • Check ava --help for more

2. Find the glob matching part of the code. Do a match on

   **/test.ts **/test-*.ts **/*.spec.ts **/*.test.ts **/test/**/*.ts **/tests/**/*.ts **/__tests__/**/*.ts

   If there are results and AVA has typescript setup, all good. Otherwise, the user might either be using tsc first, or unaware of the recipe for TS. I'm not sure how to distinguish between the two, but if the user needs education, it would be very nice to direct them towards the TS recipe with a warning.

3. When running a test file, wrap it in a try/catch and create an error catch and help tool. When catching things like window does not exists append some further text roughly stating You need to checkout link_to_browser_recipe. This catch-and-help tool could be used to add clarity to any error messages not specifically thrown by AVA.

---

What do you think? Before I work on any of this, I'd like to make sure it's relatively within your vision/aesthetic/code ethics.

[novemberborn]

1. Create a ProjectStructure.md doc which basically says

That would make for a great recipe, barring any larger documentation overhaul.

2. Find the glob matching part of the code. Do a match on

I don't think this is worthwhile. Because of point 3:

3. When running a test file, wrap it in a try/catch and create an error catch and help tool

Yes I was thinking the same thing. Pattern matching on any exceptions coming out of the test execution.

Though we could start with errors during loading: https://github.com/avajs/ava/blob/7c352db5b929ac9e59daf9dd12f6387d38639d1a/lib/worker/subprocess.js#L129

Then, if the file has a .ts extension we could provide hints regarding TypeScript setup.

[SephReed]

1. Create a ProjectStructure.md doc which basically says I really like the recipes as special project support, but I was imagining more of a "Getting started pt2" thing. From a newbie perspective, there's a missing chunk of information between the base README and test-setup.md about what to name the files and where to put them. It's all very obvious once you know though.

2. Find the glob matching part of the code. Do a match on

I think you're right about skipping this. As long as any glob ending with .ts without TS setup gives a helpful error message, that would be enough.

3. When running a test file, wrap it in a try/catch and create an error catch and help tool

This seems like the main action item. CatchHelperRepo.ts seem good?

export interface ICatchHelper {
  test: RegExp,
  helpMsg: string,
}

class _CatchHelperRepo {
  private helpers:  ICatchHelper[];

  public addCatchHelper(addMe: ICatchHelper ) {
    this.helpers.push(addMe);
  }

  public addCatchHelpers(addUs: ICatchHelper[]) {
    this.helpers = this.helpers.concat(addMe);
  }

  public addHelperMessagesToError(err: Error) {
    if (!err || !err.message) {  return; }  // TODO
    const msg = err.message;
    const helpMsgs: string[] = [ msg ];
    this.helpers.forEach((helper) => {
       if (helper.test.test(msg)) {
         helpMsgs.push(helper.helpMsg);
       }
    });
    // remove trailing period and whitespace.  Re-add it but standardized.
    err.message = helpMsgs.map((it) => it.replace(/\.\s+$/, "") + ".").join("  ");
  }
}
export const CatchHelperRepo = new _CatchHelperRepo();

---

some_main_file.ts:

import { CatchHelperRepo }  from "/path/to/file/CatchHelperRepo.ts"

try {
  runTestOnFile("/path/to/file.test.ts");
} catch (err) {
  CatchHelperRepo.addHelperMessagesToError(err);
  throw err;
}

---

some_file_near_browser_support.ts:

import { CatchHelperRepo }  from "/path/to/file/CatchHelperRepo.ts"

// for example
CatchHelperRepo.addCatchHelpers([
  { test: /window.+is undefined/;  helpMsg: "You may need to setup browser stuff, check docs link" }
  { test: /document.+is undefined/;  helpMsg: "You may need to setup browser stuff, check docs link" }
  { test: /setTimeout.+is undefined/;  helpMsg: "You may need to setup browser stuff, check docs link" }
  { test: /Event.+is undefined/;  helpMsg: "You may need to setup browser stuff, check docs link" }
]);

[novemberborn]

Sure that looks like a reasonable enough approach. We tend to avoid classes though, in favor of variables within a module and some careful exports.

[SephReed]

so basically, just flatten it?

  let helpers:  ICatchHelper[];

  export addCatchHelper(addMe: ICatchHelper ) {
    helpers.push(addMe);
  }

  export addCatchHelpers(addUs: ICatchHelper[]) {
    helpers = helpers.concat(addMe);
  }

  export addHelperMessagesToError(err: Error) {
    if (!err || !err.message) {  return; }  // TODO
    const msg = err.message;
    const helpMsgs: string[] = [ msg ];
    helpers.forEach((helper) => {
       if (helper.test.test(msg)) {
         helpMsgs.push(helper.helpMsg);
       }
    });
    // remove trailing period and whitespace.  Re-add it but standardized.
    err.message = helpMsgs.map((it) => it.replace(/\.\s+$/, "") + ".").join("  ");
  }

[novemberborn]

I wouldn't even export those add* methods.

But this discussion may be easier with a draft PR.

[SephReed]

Sounds good. Thanks for giving me an idea of what you're looking for before I start.