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

Proposal for golden output testing rule #90

Open
achew22 opened this issue Jan 4, 2019 · 9 comments
Open

Proposal for golden output testing rule #90

achew22 opened this issue Jan 4, 2019 · 9 comments
Assignees
Labels
P3 We're not considering to work on this, but happy to review a PR. (No assignee) type: feature request

Comments

@achew22
Copy link
Member

achew22 commented Jan 4, 2019

In #86 it appears the first rule is being added to skylib. Would there be any interest in adding a new rule that does golden comparison? To pick a name let's call it golden_test. I propose it is usable in 3 ways:

Literal:

golden_test(
    name = "content",
    src = ":myfile.txt", # Note that this could be the output of a genrule or other target
    content = "demonstration content for a test",
)

Regexp

golden_test(
    name = "content",
    src = ":myfile.txt", # Note that this could be the output of a genrule or other target
    regexp = "demonstration content for a .*",
)

Golden file

golden_test(
    name = "content",
    src = ":myfile.txt", # Note that this could be the output of a genrule or other target
    golden_file = ":myfile.golden.txt",
)

Would there be any interest in adding a rule like this? I believe that almost every rules_x has implemented something like this and consolidating on a single implementation seems like a not terrible idea.

CC: @c-parsons

@c-parsons
Copy link
Collaborator

Golden file verification does indeed does seem like useful functionality we may want to put into bazel-skylib. For one, Stardoc tests re-implement golden file verification from scratch, and I assume there are a number of places similar is done.

Would you mind writing a brief proposal and circulating to bazel-dev@googlegroups.com ? I'd like some user feedback before we commit to an API :) We should try to motivate the exact API with some desired usage.

We should also try to ensure the rule is platform-agnostic (and thus works for no-Bash Windows, much like the work done in #86)

@achew22
Copy link
Member Author

achew22 commented Jan 10, 2019

@c-parsons, thanks for your response.

Golden file verification does indeed does seem like useful functionality we may want to put into bazel-skylib. For one, Stardoc tests re-implement golden file verification from scratch, and I assume there are a number of places similar is done.

I know that rules_typescript, rules_closure and a few other rules have also implemented something like this, not to mention the couple that I've implemented in my own personal projects. We are well past the rule of three.

Would you mind writing a brief proposal and circulating to bazel-dev@googlegroups.com ? I'd like some user feedback before we commit to an API :) We should try to motivate the exact API with some desired usage.

What would you think of a PR as the proposal. It's nice to have a design doc but this shouldn't be extremely complex to implement and we could have a specific discussion about the exact API.

Would you mind writing a brief proposal and circulating to bazel-dev@googlegroups.com ? I'd like some user feedback before we commit to an API :) We should try to motivate the exact API with some desired usage.

I would be very happy to do that. Do you have any tips/suggestions/pitfalls that I need to look out for? I assume that Windows compatibility is the hard one to come by, but don't have a box that runs it. Any suggestions for that?

@c-parsons
Copy link
Collaborator

What would you think of a PR as the proposal. It's nice to have a design doc but this shouldn't be extremely complex to implement and we could have a specific discussion about the exact API.

That's fine -- just send a brief introduction email and link to the PR to bazel-dev to give that forum some notice there's something to look at!

I would be very happy to do that. Do you have any tips/suggestions/pitfalls that I need to look out for? I assume that Windows compatibility is the hard one to come by, but don't have a box that runs it. Any suggestions for that?

Unfortunately not my area of expertise :\ cc @laszlocsomor

@laszlocsomor
Copy link
Contributor

FYI there's already such a rule in @bazel_tools: https://github.com/bazelbuild/bazel/blob/25d2a3cd2b9701bc5c4b3d481ec0b05e8d134703/tools/build_rules/test_rules.bzl#L306

This rule doesn't work on Windows at the moment -- I tried fixing it but had to roll back: bazelbuild/bazel#6122

IMO this rule should live in skylib, not in bazel_tools. Moving it out might break existing users though. We can discuss possible approaches.

In any case I'm happy to answer question about rule development pifalls on Windows. (Which I'll hopefully get around to document once, see bazelbuild/bazel#3889.)

@achew22 if you work for Google, talk to @philwo about requesting access to Bazel team's testing Windows VM on GCP (which isn't fancy in any way, just a vanilla Windows image with Bazel and some compilers + tools already installed).

@laurentlb
Copy link
Contributor

+1 for moving file_test from @bazel_tools to skylib.
We can revisit its design at the same time, if we need.

@achew22
Copy link
Member Author

achew22 commented Jan 11, 2019

Is it okay for the core Bazel to depend on starlib? Should I move it over and add a dependency on starlib? How should I handle places where that rule is used in the core repo?

@laszlocsomor
Copy link
Contributor

We check in a copy of everything Bazel depends on, for better or worse.
So you would copy Skylib to Bazel's //third_party directory, add a local_repository rule to Bazel's WORKSPACE, and use it like so.

@tetromino tetromino added type: feature request P3 We're not considering to work on this, but happy to review a PR. (No assignee) labels Apr 17, 2021
@alexeagle
Copy link
Contributor

I think this was fixed by #136
@tetromino want to close it? (I'd be happy to sign up as a maintainer for bazel-skylib if the load is currently too high)

@alexeagle
Copy link
Contributor

Ping @tetromino ☝🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering to work on this, but happy to review a PR. (No assignee) type: feature request
Projects
None yet
Development

No branches or pull requests

6 participants