User tests: Successful: Unsuccessful:
hhvm tests in travis are taking around 4:30 in every commit to do ... basicly ... nothing.
You notices this delay clearly when there are several commits in the queue for tests: https://travis-ci.org/joomla/joomla-cms/pull_requests
This PR disables them until issues with hhvm can be solved.
Unit tests passed and hhvm takes less than 4 minutes.
None.
Category | ⇒ | Unit Tests |
Status | New | ⇒ | Pending |
Labels |
Added:
?
?
|
+1 on removing, hhvm is not official supported and if it is always failing and only takes time it doesn't makes sense
@photodude shall we remove hhvm tests until you can solve the issues?
HHVM was added as an allowed failure so that more people could get involved in development related to supporting HHVM. Same reason HHVM was added to the framework packages.
PHP 7.1 was added for the same reason, development.
Both are set as allowed failures since support is in development. The same thing was done for PHP 7.0.
Someone other than me would have to make the call about removing HHVM and IMO by the same logic PHP 7.1. @mbabker what do you think?
If we're aiming for HHVM compatibility it shouldn't be removed. If we're only going to test on known stable and supported platforms, then new versions/platforms should only be added after compatibility is confirmed. Which honestly seems bass ackwards to me (you want to demonstrate forward thinking).
Alas, I just write patches and beat on my chest these days. I gave up decision rights a long time ago.
The thing is, currently it is just extending the time travis needs.. I am absolutely for looking into new platforms and we should invest way more in (automated) testing.
But always failing tests don't bring any benefit, except as you want them as constant reminder that we have something to do there :-)
I agree with the "forward thinking" @mbabker @photodude and IMO it's great you guys are testing joomla in hhvm and 7.1.
I thing we can all agree with that.
The only question for me is: instead of having this tests merged before they are working, can't we have two PR to do that testing and be merged when ready?
One PR will check PHP 7.1 tests and another HHVM tests, when tests are ok, they get merged.
I don't see any issue with this, and with this we don't have to test in every commit things that we all know it will fail.
But like mbabker, just "beating on my chest"
@andrepereiradasilva do you mean separate branches? That would be the best way to have automated testing for indevelopment stuff and not in staging. Technically, PR's should be submitted when they are ready for review, testing, and merge consideration (noting that PRs often have revisions on review).
Personally, I think separate branches would only work if there is a group of people working actively on the development. Otherwise the branches are more than likely going to be ignored and end up with merge conflicts.
For HHVM I think I might be the only one trying to solve the problems and the test blocking issues. Same with the coding standards using phpcs2.x. With just one person semi-actively working on these, and truthfully only sporadically working in these, it will be a while for support to be implemented.
Technically, PR's should be submitted when they are ready for review, testing, and merge consideration (noting that PRs often have revisions on review).
True, but how can you test travis otherwhise? If i'm right, you can either have a PR, a branch or a repository to test.
I think, from a mantainance perspective, a PR is much more simple, so IMHO it could be a exception to this. But yes, one could also have a branch or a repository somewhere to test this travis things.
can't we have two PR to do that testing and be merged when ready?
The problem with that is the maintenance team has already established that WIP PRs aren't favored (they should be in a fully testable state). So unless we're making exceptions for that, it's already a no-go. Next is those PRs would have to be manually sync'd with staging so whomever is "owning" that process goes through a lot more work. Not to mention that now only committed code gets tested against the new environments whereas with the current setup every change gets tested on environments where support is still a WIP. For me personally there's no benefit in maintaining a PR for unit testing support on PHP 7.1 that'll never be merged because the odds of us getting PHPUnit 4.8 (which isn't supported on PHP 7.x) to run on Travis on that PHP branch right now are slim to none, and since we can't drop < PHP 5.6 support working with PHPUnit 5.x isn't an easy option. And no, I don't want to hear any suggestions about a separate test suite for PHPUnit 4.8 and 5.x.
Is there really some pull request that gets held up by the extra few minutes it takes to run the PHP 7.1 and HHVM builds? Nothing is getting merged with a 5-minute turnaround rate and I don't see that becoming a goal, so really what we're trying to optimize it seems is when people are editing single files via GitHub's UI and cause 10 builds to stack up in the process. Any project which takes contributions has that issue, and honestly we've already got one of the "slimmest" Travis builds out there. Our issue is right now we're unit testing for no regressions on 5 stable PHP branches, running a build for PHPCS, a build for JavaScript integration testing, and two additional builds for unit testing WIP platform support. It'd get a lot worse if we ever had a system testing suite to run. Unless you limit what builds run on pull requests and only run certain things as they're merged to staging or a version branch, or we start taking shortcuts and not test all platforms, there's no getting around how many resources we're using. Those two builds for WIP support IMO aren't a major issue here.
ok so we have two differents opinions here.
so mantainers, close this or merge this.
closing, with the hope that we get hhvm running soon, thanks all for the discussion and working on this.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-08-25 05:10:06 |
Closed_By | ⇒ | rdeutz |
@andrepereiradasilva then remove all lines for hhvm completely. There is no reason to keep it in the .travis.yml then imo..