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

Standalone Jsonnet integration test suite #141

Open
davidzchen opened this issue Mar 25, 2016 · 10 comments
Open

Standalone Jsonnet integration test suite #141

davidzchen opened this issue Mar 25, 2016 · 10 comments

Comments

@davidzchen
Copy link
Contributor

Now that a pure-Go implementation of Jsonnet is being developed (google/go-jsonnet) is being developed, it would be a good idea to make it possible to share the Jsonnet's rich test suite across different implementations.

Perhaps we can move the test suite into a separate repo and set it up so that it can be used to test any Jsonnet implementation that exposes the same command line interface. It should be possible to set this up using Bazel. We already have a jsonnet_to_json_test rule that can be used to run all of the integration tests aside from the formatter tests, for which it would be easy to write a test rule for. We can then use Bazel's external repository mechanism to include this test suite and add the test targets.

Here is how I imagine this will work:

Say that we move the test suite to the repository google/jsonnet-test-suite. We can add a Skylark macro called jsonnet_test_suite that would take a label for a Jsonnet command line binary as an argument. When this macro is used, it will add a bunch of jsonnet_to_json_test test targets that would be run with the given Jsonnet binary.

Then, in google/jsonnet and google/go-jsonnet, we can include the google/jsonnet-test-suite repository by adding the following to the WORKSPACE file:

git_repository(
    name = "jsonnet_test_suite",
    remote = "https://github.com/google/jsonnet-test-suite",
    tag = "v0.0.1",
)

Say that //cmd:jsonnet is the build label for the jsonnet binary. Then, we can add a BUILD file under the test_suites directory with the following:

load("@jsonnet_test_suite//:test_suite.bzl", "jsonnet_test_suite")

jsonnet_test_suite(
    name = "test_suite",
    jsonnet = "//cmd:jsonnet",
)

Then, the entire test suite can be run against the given jsonnet binary with bazel test //test_suite:all.

@sparkprime
Copy link
Contributor

@jbeda probably has some thoughts on this.

All this SGTM and I don't mind using bazel for this. It is kinda a barrier to entry for people wanting to make changes and run the tests locally, but there aren't that many such people and it's not that hard to install it (unless you're on a Google Linux machine, ironically).

@davidzchen
Copy link
Contributor Author

Are you referring to making changes to the tests themselves? We can also add build rules to the test suite so that you can just run the test suite with e.g. the jsonnet binary installed on your system.

If you want to test changes to both the test suite and to jsonnet simultaneously, then you can change the git_repository workspace rule to a local_repository. Ideally, it would be great to just pass a command line flag to the bazel command, but I don't think workspace rules are configurable and it would be good to file a good feature request for this to Bazel.

Let's discuss offline to sort out the issues you're having with installing Bazel on your Goobuntu machine. FWIW, I think it is due to the fact that installing binaries to system directories without using apt-get is generally discouraged, but now that we have a Homebrew recipe for Bazel, you should be able to install that using Linuxbrew.

@jbeda
Copy link
Contributor

jbeda commented Mar 28, 2016

Moving the test suite out makes a ton of sense. However, I have to agree with @sparkprime that requiring Bazel will feel unnatural and be a barrier for many people. I'd request that there be a way to run the test suite without getting bezel installed and working.

@sparkprime
Copy link
Contributor

If installing bazel on majority Linux and OSX systems is a single one-liner, and it's not intrusive beyond that point, then I would be OK with it. I think it may be at that point now? But there were some problems with newer JDK requirements and lack of packaging of Bazel. If we can document the one-liner required to install it in this case, that would be fine. However if we require bleeding edge Bazel for some reason, that would be very bad.

@davidzchen
Copy link
Contributor Author

Installing Bazel is basically a one-liner now. You can either run the installer script or use the .deb. There is also a Homebrew recipe for Bazel as well. We definitely wouldn't need to use HEAD. :)

Here is the releases page, which provides the installer scripts and .deb packages: https://github.com/bazelbuild/bazel/releases

The only thing that would make this even easier would be to get Bazel into Debian main, but we are currently blocked some of Bazel's dependencies. I also know some people who are Debian package maintainers, and I might help out with pushing this forward.

@sparkprime
Copy link
Contributor

I just tried the deb and it errored out:

dcunnin@tyrion:~/Downloads$ sudo dpkg -i bazel_0.2.0-jdk7-linux-x86_64.deb 
[sudo] password for dcunnin: 
Selecting previously unselected package bazel.
(Reading database ... 291789 files and directories currently installed.)
Preparing to unpack bazel_0.2.0-jdk7-linux-x86_64.deb ...
Unpacking bazel (0.2.0~-jdk7) ...
dpkg: dependency problems prevent configuration of bazel:
 bazel depends on java7-jdk; however:
  Package java7-jdk is not installed.

dpkg: error processing package bazel (--install):
 dependency problems - leaving unconfigured
Errors were encountered while processing:
 bazel

I have jdk7 installed via openjdk-7-jdk

@davidzchen
Copy link
Contributor Author

Can you open a bug on the Bazel issue tracker for that?

@jbeda
Copy link
Contributor

jbeda commented Mar 30, 2016

Please don't require Bazel. It should be possible to run a test suite without installing a beta quality build tool.

@sparkprime
Copy link
Contributor

Yeah I think we're better off modifying the existing scripts to fetch the tests from the other repo using git submodules or something like that. You can of course build this out on Bazel as well, and we can switch to that when it's ready. I'm all for supporting the other open source efforts. But until I can install it as easily as make / vim / emacs then it shouldn't be on the required path.

@davidzchen
Copy link
Contributor Author

Sounds good to me. The main reason why I suggested Bazel is that it already provides a lot of the tools and mechanisms in place that make it easy to set up and run this kind of shared test suite with less boilerplate than if we were to script everything ourselves. Of course, I am not saying that we should require that this be the only way to run the test suite and would be fine with also providing a set of scripts for those who are using other build systems to build their implementation of Jsonnet.

In the meantime, I'll also take a look at what is left to do before we can get Bazel into Debian main and try to move that forward.

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

No branches or pull requests

3 participants