packaging
packaging copied to clipboard
hhvm
package test runner in hhvm-dev packages
It'd be nice if the hhvm test runner was packaged in hhvm-dev, so extension testsuites could be run after building them.
just let me know if this is something that you folks want to do. I was gonna include this in my fedora package, but there's no point if it doesn't get any buy in from the ubuntu/debian packagers.
I am surprised that the test runner isn't packaged? :/
cc: @jwatzman @fredemmott
hhvm-dev is for separate extensions right, which wouldn't get any utility out of our testsuite -- though I guess you could break core functionality with them?
Also, would the runner be useful without the tests, which I don't think it makes sense to package? Or do some extensions have test suites that use our test runner but test their extension specifically?
Am I missing something? (Totally possible :-P)
shouldn't extensions use the same test runner? Or should they all pick their own? It doesn't actually have to be in hhvm-dev, but i do think it should be packaged somehow, because i don't think extensions should have to come up with their own test runner.
a specific usecase would be the hhvm-pgsql extension. I plan on bringing in the php tests over for pgsql and pdo_pgsql to that repo, but we need a test runner to be able to run them. I'd like to avoid a manual copy of the test runner if possible.
Hm yeah I see what you're trying to do now, but I'm still not sure that hhvm-dev is the right place to contain the runner. But it also probably doesn't hurt anything. Strong feelings, anyone else?
I think making it available is a good idea, I don't have any strong feelings about where it goes though. hhvm-dev seems like a reasonable place though, given it's intended usage.
@jwatzman : do you happen to have any better ideas? i think a separate package is overkill personally, but maybe that's just me.
OK, I'm convinced. Happy to take a PR for this, should be a one-line change to the packaging script, otherwise will do it when I get a chance.
i might be able to submit a PR to the debian package, it's probably easy to figure out. The question is, what do we name it? run is not a good name.
hhvm-test-runner ?
i wonder if the shebang line at the top of the test runner should be changed to #!/usr/bin/env hhvm instead of #!/usr/bin/env php
because atm you'd have to call it explicitly with hhvm /path/to/hhvm-test-runner tests/
We do now run a subset of tests using php5 to execute the runner, so that should hopefully be unneccessary
Yeah either PHP5 or HHVM should work for the runner. hhvm-test-runner works for me, hhvm-run-tests is also fine, something like that.
ok, two alternatives:
- a shell script that finds hhvm and calls the runner
- a cmake target so you can run make test
I'm not sure I follow -- I thought you were just going to copy the existing script into the dev package somewhere? Do you need something else? What am I not understanding? :)
because to excecute the copied script it will require a call like this:
hhvm /path/to/hhvm-test-runner tests
when it should be:
hhvm-testrunner tests
because the test runner has a shebang line of #!/usr/bin/env php (not hhvm) which means it won't automatically execute hhvm.
EDIT: the alternative being able to make a test target in cmake that gets included in the dev CMake files, then you can just run
make test
and that target will iron out the differences by trying to find hhvm itself.
because the test runner has a shebang line of #!/usr/bin/env php (not hhvm) which means it won't automatically execute hhvm.
This is deliberate, the test runner itself is compatible with PHP5; this has nothing to do with which HHVM it is currently testing (it makes some effort to locate HHVM to test, and will notably just use which hhvm if it detects it's testing an out-of-tree extension). It doesn't matter which runtime the runner itself is using. Or, again, am I missing something?
yes, i got that based on what fred said. but since it uses php, then it can't find hhvm by itself when you invoke it. It's too late to try to find it later. so you must invoke with with the hhvm executable as:
hhvm hhvm-test-runner
instead of just:
hhvm-test-runner
It will run which hhvm to find HHVM https://github.com/facebook/hhvm/blob/d6577daf89598be8bdaa42d7489950b7fd8da265/hphp/test/run#L117-L118 -- why isn't that sufficient?
because it's too late by then. the point is calling the test-runner itself with hhvm for regular installs. Unless we're operating under the assumption that php must also be installed alongside hhvm everytime.
we're operating under the assumption that php must also be installed alongside hhvm everytime
Ah that's what I missed, if you don't have php the runner may not find a runtime to begin executing with. I think saying that we require PHP5 for this is kinda silly :)
Hm, kinda ugly to just use sed or something to switch it over during install, but I don't think we want to change it over to hhvm for internal purposes? @fredemmott is it possible to do it the other way, what makes sense to you here?
@jwatzman @jrobeson Is the point here to be able to run the test runner with only hphp/test/run with no php/hhvm prefix AND no arguments to the runner?
https://reviews.facebook.net/D31215 should be landing soon which, as a small part of it, provides an option to specify the hhvm executable as an argument to the runner (the diff has changed somewhat from what you see in the link as most of the comments and changes have been internal), but the new argument is still there.
@JoelMarcey : the point is to provide the hhvm test runner to third party extensions, so they can run their own tests without copying the test runner into their own repositories. However, the test runner has a shebang line that points to a php executable, which won't exist when installing hhvm-dev(el) packages without php also being installed.
Internal usage for hhvm doesn't have to change.
EDIT: @jwatzman clarified that the debian packages use update-alternatives to set hhvm as a possible php executable, but not all distributions use (or want to use) such a mechanism
@jrobeson Yeah, I just realized that this about how to actually begin running the thing which the diff I referenced doesn't deal with at all.