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

feat(cli): support configuring the test tool in the config file #15079

Merged
merged 17 commits into from
Jul 18, 2022
Merged

feat(cli): support configuring the test tool in the config file #15079

merged 17 commits into from
Jul 18, 2022

Conversation

rojvv
Copy link
Contributor

@rojvv rojvv commented Jul 5, 2022

This allows the user to use the config file to configure the included and excluded patterns of the test subcommand.

It has many use cases, one of them is for example when the project uses dnt to transform the tests to work on Node.js, and place them in a directory called out/. The test tool should not look for test files in that directory, and instead of providing the CLI args to exclude that directory everytime, the user can just have a config file like the following:

{
  "test": {
    "files": {
      "exclude": ["./out/"]
    }
  }
}

This is just a single example, there might be more examples that will define the usefulness of the existence of this feature.

This allows the user to use the config file to configure the included and excluded patterns of the test subcommand.

It has many use cases, one of them is for example when the project uses dnt to transform the tests to work on Node.js, and place them in a directory called `out/`. The test tool should not look for test files in that directory, and instead of providing the CLI args to exclude that directory everytime, the user can just have a config file like the following:

```jsonc
{
  "test": {
    "files": {
      "exclude": ["./out/"]
    }
  }
}
```

This is just a single example, there might be more examples that will define the usefulness of the existence of this feature.
@rojvv rojvv changed the title feat: configuring test files in the config file feat: configuring the test tool in the config file Jul 5, 2022
@dsherret
Copy link
Member

Related: #14263

Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks almost good other than the LSP change. I also think this would be nice for the reason you mentioned.

cli/lsp/language_server.rs Outdated Show resolved Hide resolved
@bartlomieju bartlomieju added this to the 1.24 milestone Jul 12, 2022
@rojvv
Copy link
Contributor Author

rojvv commented Jul 13, 2022

@dsherret Mind another review?

@bartlomieju
Copy link
Member

We've discussed this PR at the last CLI design meeting and we're in agreement this is useful.

Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @roj1512!

Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait, I just realized there's no tests for this. We'll need some integration tests before this lands (see lint/fmt tests for an example in cli/tests/integration).

@rojvv rojvv changed the title feat: configuring the test tool in the config file feat(cli): support configuring the test tool in the config file Jul 16, 2022
@rojvv
Copy link
Contributor Author

rojvv commented Jul 16, 2022

@dsherret I added the include test. The last failed CI is not related to my changes.

@rojvv rojvv requested a review from dsherret July 18, 2022 12:25
Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @roj1512—this will be really nice to have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider adding config for deno test to deno.jsonc
3 participants