[FR]: How to integrate custom test rules with configure

[antspy]

What is the current behavior?

Hi,

I am using rules_ts with bazel, and by default it will create a ts_project rule for each .ts file found in a directory. The same applies to test files ending in .spec.ts..

This behavior can be modified to a limited extent using gazelle directives, but I don't see a way to get the behavior I want.

Describe the feature

I have a custom rule I want to use for tests. Nothing special, but it combines a ts_project for the test file with a vitest run from load("@npm//:vitest/package_json.bzl", vitest_bin = "bin").

So the idea is that I would be able to write

ts_test(
    name = 'mytest', 
    srcs = ['my.test.ts'],
    deps = [...], 
)

and that's all. I don't see how to get the behavior I want though.

1. I cannot use a simple aspect:map_kind directive, because I want the substitution ts_project -> ts_test to happen only for test files, not for every filetype (other ts files should still use the ts_project rule).
2. I looked into the Starlark Extension API (which seems really cool!) but I have found 2 issues:
   • The first is that the default ts_project behavior is not changes, so now I have both a ts_test rule and a ts_project rule, which is not what I want
   • The second is that I would have to write my own logic for ts_test dependencies. Here I just want to reuse the same logic from ts_project, just set in a different macro - but I don't want to rewrite the whole 'parse file import to add dependencies' logic.

So my question is - what can I do about this? I am very new to this space, so perhaps there's something obvious I am missing.
One thing that I think might work is to add a gazelle directive to specify which rule should be used for the .spec.ts targets, so that I could set it to my own rule using the aspect:map_kind directive and have everything else work as it is, without having to write a custom starlark plugin

Another idea would be exporting the whole logic as a starlark extension so one could change it without having to fork / recompile the CLI, but directly adding a new plugin.

Any help would be appreciated. Thanks a lot!

[jbedard]

Ideally all you should need is a starlark extension that generates the vitest target and takes the pre-compiled .spec.js files as inputs. Your vitest rule should not do anything typescript related.

See #439 for the map_kind issue. I'm open to API suggestions there if you have some, and the more requests for a solution there the more likely I can put time into it 😅

As a workaround you can do for now... use map_kind to use a custom ts_project macro, then within that macro essentially do:

def ts_project(name, **kwargs):
   if kwargs.get("testonly", False): # or instead match on 'name' if you have multiple testonly targets
       ts_test_project_macro(name, **kwargs)
   else:
       ts_normal_project_macro(name, **kwargs)

[antspy]

Hi @jbedard ,

Thanks a lot for your answer! :)

Ideally all you should need is a starlark extension that generates the vitest target and takes the pre-compiled .spec.js files as inputs. Your vitest rule should not do anything typescript related.

I can do this, but then I end up with two ts_project in my build file, which tends to confuse aspect configure. For example, this is the end state I want:

ts_project(
    name = "sample",
    srcs = ["sample.ts"],
    deps = [
        "//:node_modules/@types/node"
    ],
)

ts_test(
    name = "sample_test",
    size = "small",
    srcs = ["sample.test.ts"],
    deps = [
        ":sample",
        "//:node_modules/@types/node",
    ],
)

and I want aspect configure to automatically add ts_test targets when they are missing, and update the deps automatically. The ts_test macro generates an internal ts_project and has a vitest_test target which depends on the internal ts_project rule.

Following your suggestion, I could add a custom plugin that only generates the vitest_test target, and leave the automatic ts_project alone, resulting in something like this:

ts_project(
    name = "sample",
    srcs = ["sample.ts"],
    deps = [
        "//:node_modules/@types/node"
    ],
)

# gazelle:js_project_naming_convention sample_test_lib
ts_project(
    name = "sample_test_lib",
    srcs = ["sample.test.ts"],
    deps = [
        "//:node_modules/@types/node"
    ],
)

vitest(
    name = "sample_test",
    size = "small",
    srcs = ["sample.test.ts"],
    data = [
        ":sample_test_lib",
    ],
)

But this doesn't seem a great solution, because

1. aspect configure by default expects a single ts_project per directory, so I need to add manually the directive # gazelle:js_project_naming_convention sample_test_lib to avoid configure complaining that the target already exists.
2. If I import the sample library in other files, configure gets confused that there is more than one ts_project in the build. I get errors like " “Import “../../sample.js” from “xxx” resolved to multiple targets (//…:sample, //…:sample_lib_test)”". So how would I disambiguate between the two?
3. Having an extra ts_project adds boilerplate for no reason. Ideally we would have something like in python - there is a py_library, a py_binary, and a py_test. The less boilerplate we have in BUILD files the better.

Please let me know what you think! :)

See #439 for the map_kind issue. I'm open to API suggestions there if you have some

yes that's exactly what I want :) I agree with the suggestion in the FR, you can add a different kind generated for .spec.ts files, which then would allow us to use map_kind and use our custom macros. This seems relatively low effort to support and would allow us to have our own macros used for testing, which is pretty cool.

Perhaps the best solution long term is to move away from pre-compiled binaries (removing the go lib entirely) but provide libraries for custom plugins to configure (https://docs.aspect.build/cli/starlark) - so one can have full control over the generated targets.

Because essentially, the real hard things that configure does is automatically adding the dependency. Then all the rest of it (naming, auto-creating the targets, etc.) is sort of useful but not that big of a deal - you can get by very easily just copy pasting.[0] It's dependencies which are a pain and need configure to be updated. So just adding a starlark library that takes in input a file and returns a list of dependencies would be great, and then one can easily configure naming and target type generation just by changing the declare_targets function.

[0] In fact, I find configure to be too opinionated in the naming of the targets. I would argue that if I target already exists, it should not rename it or try to add extra targets. If I could only enable configure as a way to add dependencies and nothing else (build_cleaner style), I would - I don't really need automatic target generations.

[jbedard]

1. aspect configure by default expects a single ts_project per directory, so I need to add manually the directive # gazelle:js_project_naming_convention sample_test_lib to avoid configure complaining that the target already exists.

By default it actually has 2 ts_projects per directory: lib + tests. You can override the default names and globs for those targets, you can also add custom targets (such as e2e tests etc) with additional names + globs.

2. If I import the sample library in other files, configure gets confused that there is more than one ts_project in the build. I get errors like " “Import “../../sample.js” from “xxx” resolved to multiple targets (//…:sample, //…:sample_lib_test)”". So how would I disambiguate between the two?

A source file should only be in one ts_project target, otherwise it makes sense that it can't resolve to a single target.

3. Having an extra ts_project adds boilerplate for no reason. Ideally we would have something like in python - there is a py_library, a py_binary, and a py_test. The less boilerplate we have in BUILD files the better.

What do you consider to be "extra"? Each .ts should be in a single ts_project. If you're writing non-generated ts_projects then those sources should be excluded from the generated ones, but I'd hope you don't need anything non-generated.

[0] In fact, I find configure to be too opinionated in the naming of the targets. I would argue that if I target already exists, it should not rename it or try to add extra targets. If I could only enable configure as a way to add dependencies and nothing else (build_cleaner style), I would - I don't really need automatic target generations.

You can name the targets whatever you want and provide whatever source globs you want, see https://docs.aspect.build/cli/help/directives/#javascript

While configure can understand the outputs of any target, it can only generate attributes for targets it generates. So unfortunately if you want ts_project(deps) generated then the full ts_project needs to be generated.

This seems relatively low effort to support and would allow us to have our own macros used for testing, which is pretty cool.

I haven't looked at this in a long time but from what I can recall it wasn't that easy because map_kind is implemented within the gazelle core and was awkward to customize or patch. I think short term it would be easier for you to detect lib vs tests by name like I suggested, but ideally you wouldn't have to do this at all.