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

Add support for the yarn test -f [externalPackage] command #7889

Closed
ma2ciek opened this issue Aug 20, 2020 · 4 comments · Fixed by ckeditor/ckeditor5-dev#667
Closed

Add support for the yarn test -f [externalPackage] command #7889

ma2ciek opened this issue Aug 20, 2020 · 4 comments · Fixed by ckeditor/ckeditor5-dev#667
Assignees
Labels
package:dev squad:platform Issue to be handled by the Platform team. type:feature This issue reports a feature request (an idea for a new functionality or a missing option).

Comments

@ma2ciek
Copy link
Contributor

ma2ciek commented Aug 20, 2020

A follow-up to the environment unification. I feel we should add support for yarn test -f [externalPackage] when the externalPackage is cloned to the external directory.

cc @pomek


If you'd like to see this feature implemented, add a 👍 reaction to this post.

@ma2ciek ma2ciek added type:feature This issue reports a feature request (an idea for a new functionality or a missing option). package:dev labels Aug 20, 2020
@pomek
Copy link
Member

pomek commented Aug 20, 2020

It works when you specify a package name. However, (I guess) the ticket is about supporting an option to test all packages inside an external repository.

For the following tree:

packages/
    ...
    ckeditor5-xyz
external/
    custom-repository/
        packages/
            ckeditor5-foo/
            ckeditor5-bar/

The following command:

yarn run test -f custom-repository

Should be translated to:

yarn run test -f foo,bar

At this point, I would like to rename the requested option name: --repository or as an alias -r which, if specified, reads all directories inside the packages/ directory and passes it as additional values for the --files option.

So, if you would like to execute all custom-repository packages and xyz, you could type:

yarn run test -f xyz -r custom-repository

And it will be resolved as:

yarn run test -f xyz,foo,bar

@ma2ciek
Copy link
Contributor Author

ma2ciek commented Sep 1, 2020

This is a part I certainly don’t want to overcomplicate by introducing new resolution semantics etc. I like your proposal, on the other hand, the implementation becomes pretty ugly with our strategy of stripping the ckeditor5-  part whenever possible 😄

Maybe we could use -e instead of -r as a shortcut from --external? On the other hand, we could enhance the -r option to support yarn test -r ckeditor5 as well (IDK if this makes sense).

@pomek
Copy link
Member

pomek commented Sep 1, 2020

we could enhance the -r option to support yarn test -r ckeditor5 as well (IDK if this makes sense).

I was thinking about it as well and it makes sense to have an option to test all packages from the root directory without removing external stuff.

@mlewand
Copy link
Contributor

mlewand commented Sep 1, 2020

An option to run tests per repository seems to be fine 👍

@mlewand mlewand added the squad:platform Issue to be handled by the Platform team. label Sep 1, 2020
ma2ciek added a commit to ckeditor/ckeditor5-dev that referenced this issue Sep 2, 2020
Feature (tests): Introduced the --repositories (also known as -r) that allows specifying a name of a repository (or repositories, separated by a comma, similar to the --files option) where the tool should look for packages that should be tested. Thanks to that, you do not have to specify all packages of a repository that was cloned into the external/ directory. Closes ckeditor/ckeditor5#7889.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package:dev squad:platform Issue to be handled by the Platform team. type:feature This issue reports a feature request (an idea for a new functionality or a missing option).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants