? ? Success

User tests: Successful: Unsuccessful:

avatar andrepereiradasilva
andrepereiradasilva
23 Aug 2016

Summary of Changes

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

image

This PR disables them until issues with hhvm can be solved.

Testing Instructions

Unit tests passed and hhvm takes less than 4 minutes.

Documentation Changes Required

None.

4e7b59c 23 Aug 2016 avatar andrepereiradasilva ups
avatar joomla-cms-bot joomla-cms-bot - change - 23 Aug 2016
Category Unit Tests
avatar andrepereiradasilva andrepereiradasilva - open - 23 Aug 2016
avatar andrepereiradasilva andrepereiradasilva - change - 23 Aug 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 23 Aug 2016
Labels Added: ? ?
avatar andrepereiradasilva andrepereiradasilva - change - 23 Aug 2016
The description was changed
6bc42ae 24 Aug 2016 avatar andrepereiradasilva ups
avatar yvesh
yvesh - comment - 24 Aug 2016

@andrepereiradasilva then remove all lines for hhvm completely. There is no reason to keep it in the .travis.yml then imo..

avatar rdeutz
rdeutz - comment - 24 Aug 2016

+1 on removing, hhvm is not official supported and if it is always failing and only takes time it doesn't makes sense

avatar andrepereiradasilva
andrepereiradasilva - comment - 24 Aug 2016

@photodude shall we remove hhvm tests until you can solve the issues?

avatar photodude
photodude - comment - 24 Aug 2016

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?

avatar mbabker
mbabker - comment - 24 Aug 2016

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.

avatar yvesh
yvesh - comment - 24 Aug 2016

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 :-)

avatar photodude
photodude - comment - 24 Aug 2016

@mbabker thank you for your input. I personally agree with your comments.

avatar andrepereiradasilva
andrepereiradasilva - comment - 24 Aug 2016

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" ?

avatar photodude
photodude - comment - 24 Aug 2016

@yvesh part of the reason that the HHVM tests take longer is due to those test not being able to run on travis's precise container. Lots of people are still waiting for Travis to make a trusty container for faster testing.

avatar photodude
photodude - comment - 24 Aug 2016

@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.

avatar andrepereiradasilva
andrepereiradasilva - comment - 24 Aug 2016

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.

avatar mbabker
mbabker - comment - 24 Aug 2016

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.

avatar andrepereiradasilva
andrepereiradasilva - comment - 24 Aug 2016

ok so we have two differents opinions here.

so mantainers, close this or merge this.

avatar rdeutz
rdeutz - comment - 25 Aug 2016

closing, with the hope that we get hhvm running soon, thanks all for the discussion and working on this.

avatar rdeutz rdeutz - change - 25 Aug 2016
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2016-08-25 05:10:06
Closed_By rdeutz

Add a Comment

Login with GitHub to post a comment