This seems like a lot of complexity to manage, things that could break, etc.

Can we get away with simply using #4189 instead?

If we want to run @github-api tests in CI, we could do so by only running them in one job and setting a GITHUB_TOKEN to increase our rate limit to 5000

Hi (was away for several days)

This seems like a lot of complexity to manage, things that could break, etc.
Can we get away with simply using #4189 instead?

Yes, I agree - that's a much simpler solution.

If we want to run @github-api tests in CI, we could do so by only running them in one job and setting a GITHUB_TOKEN to increase our rate limit to 5000

I'll give that a try tomorrow (it wasn't/isn't clear to me that it works), so I'll close this PR.

Using a token seems to work fine - see gitlost#13, running all jobs (actually I left out PHP 7.1 but that was a mistake), if slowly (a nice side-effect of using a cache now lost was speed).

If you want to go ahead with this I can do a PR. If you want to it just in one job I suppose PHP 7.0 as the most "mainstream" currently? Also I created a personal token WP_CLI_GITHUB_TOKEN on my fork so I suppose you (or is there a "wp-cli" user?!) would be best to do that on this repo.

Actually getting rate limiting errors on trying it today so either I'm doing it wrong or it doesn't work.

Actually getting rate limiting errors on trying it today so either I'm doing it wrong or it doesn't work.

Ok. We'll go ahead and punt again on this for now. If it becomes a pressing issue in the future, we can revisit it.

Got a chance to properly debug it and quelle surprise I was doing it wrong - had the format wrong in travis.yml and wasn't exporting it in get_process_env_variables() and the http_request() args were wrong anyway.

Anyway after testing with the fixes it does work, so I think this PR is now a runner seeing as it no longer needs the "primed cache" hack.

@gitlost I've added a GITHUB_TOKEN environment variable with a token. Can you submit a new PR that solely enables GitHub API tests for one build, and uses the GITHUB_TOKEN in the request when it's present as an environment variable?

Do you think the environment variable should be GITHUB_TOKEN or WP_CLI_GITHUB_TOKEN? Also could you post the output of (as appropriate)

travis encrypt WP_CLI_GITHUB_TOKEN=XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -r wp-cli/wp-cli

please?

The current plan is to replace github-api in features/cli.feature with a new tag travis-github-token (as github-api is still being used in package-command/features/package-install.feature:123) and to add to ci/behat-tags.php

# Skip Github API tests that require WP_CLI_GITHUB_TOKEN if on Travis and it's not available.
if ( getenv( 'TRAVIS' ) && ! getenv( 'WP_CLI_GITHUB_TOKEN' ) ) {
	$skip_tags[] = '@travis-github-token';
}

I've veered back to thinking that caching (and paging) should be done with or without a token. It respects what the api.github.com/repos request is saying - that the data is good (and should be cached) for max-age seconds. So I think this PR is the appropriate place to do things (with the prime cache stuff (and the unrelated remove directory fix) removed).

Do you think the environment variable should be GITHUB_TOKEN or WP_CLI_GITHUB_TOKEN

GITHUB_TOKEN is a standard (e.g. Composer).

Also could you post the output of (as appropriate)

It doesn't need to be included in .travis.yml because I entered it through the Travis backend.

So I think this PR is the appropriate place to do things (with the prime cache stuff (and the unrelated remove directory fix) removed).

I don't really want to include caching at this point. It adds complexity to the build system, and therefore the likelihood of something later breaking.

Maybe we should simply continue to skip these tests in CI. I suspect there are more important things for us to work on.

I entered it through the Travis backend

Okay, so apply it to all jobs then?

It adds complexity to the build system

In what way?

I suspect there are more important things for us to work on.

I think getting tests working fully and speedily on travis and locally, with as few exceptions as possible and with no extraneous errors, is a very important thing to be working on, and will improve all our lives! The slowness is a particular millstone, and having exceptions or errors is confusing and leads one to miss bugs.

In what way?

It's additional code executed, with additional execution paths, which increases the overall complexity of the system. Notably, caching is notorious for introducing issues that are only intermittently reproducible.

I think getting tests working fully and speedily on travis and locally, with as few exceptions as possible and with no extraneous errors, is a very important thing to be working on, and will improve all our lives!

Right. Even within scope:testing, there are a number of issues that will yield higher impact. For instance, #4108 is arguably critical and much higher priority than this.

I don't mean to discount your work on this. I appreciate the effort invested so far. I just mean to suggest there might be more fruitful investments of the limited amount of time you have.

It's additional code executed

You could say that about anything!

Notably, caching is notorious for introducing issues that are only intermittently reproducible

I can't see that applying in this case - either the cache is there or it isn't. If it isn't it doesn't matter. If it is, it's checked and only then used. It's pretty fail-safe I hope (famous last words).

a number of issues that will yield higher impact

I'm sure there are! I wasn't claiming otherwise or making comparisons (which are always odious).

#4108 is arguably critical and much higher priority than this

I'm sure it is! Alain's doing that though isn't he?

I don't mean to discount your work on this

I'm sure you don't. I'm not emotionally attached to this PR. If you're set against it, I'll drop it. I just think it's the right thing to do.

It's pretty fail-safe I hope (famous last words).

In looking back at #1612 to show you how much time I've spent on this already, I completely forgot I had written a cache implementation #1618 that I ended up reverting #1653.

Really, I'm just trying to save you (and myself) from trodding this already trodden path and potentially experiencing a world of pain along the way.

If this breaks again, we can seriously explore running the @skip-github-api tests in CI. I just don't think it's important enough to spend the time on now.

Cool. Consider it dropped!