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

Add support for Github Actions #275

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 45 additions & 0 deletions .github/workflows/blank.yml
@@ -0,0 +1,45 @@
name: CI

on:
push:
branches:
tags:
pull_request:

jobs:
build:
runs-on: ubuntu-latest
strategy:
matrix:
php: [7.1, 7.2, 7.3]
steps:
- uses: actions/checkout@v1
- name: Debug if needed
run: |
export DEBUG=${DEBUG:-false}
if [[ "$DEBUG" == "true" ]]; then
env
fi
env:
DEBUG: ${{secrets.DEBUG}}
- name: Setup php
uses: shivammathur/setup-php@1.7.0
with:
php-version: ${{ matrix.php }}
extension-csv: dom, mbstring
coverage: xdebug
- name: Get Composer Cache Directory
id: composer-cache
run: echo "::set-output name=dir::$(composer config cache-files-dir)"
- name: Cache dependencies
uses: actions/cache@v1
with:
path: ${{ steps.composer-cache.outputs.dir }}
key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }}
restore-keys: ${{ runner.os }}-composer-
- name: Install dependencies
run: composer install
- name: Run composer script
run: |
composer install-dev-tools;
composer sca
43 changes: 43 additions & 0 deletions src/Bundle/CoverallsBundle/Collector/CiEnvVarsCollector.php
Expand Up @@ -63,6 +63,7 @@ public function collect(array $env)
->fillCircleCi()
->fillAppVeyor()
->fillJenkins()
->fillGithubActions()
->fillLocal()
->fillRepoToken();

Expand Down Expand Up @@ -110,6 +111,48 @@ protected function fillTravisCi()
return $this;
}

/**
* Fill Github Actions environment variables.
*
* @return $this
*/
protected function fillGithubActions()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to keep the method order to comply with order in collect().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean that call fillGithubActions should last?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

{
if (!isset($this->env['GITHUB_ACTIONS'])) {
return $this;
}
$this->env['CI_NAME'] = 'github';

$githubEventName = $this->env['GITHUB_EVENT_NAME'];
$githubSha = $this->env['GITHUB_SHA'];
$githubRef = $this->env['GITHUB_REF'];

if (strpos($githubRef, 'refs/heads/') !== false) {
$githubRef = str_replace('refs/heads/', '', $githubRef);
$jobId = $githubSha;
} elseif ($githubEventName === 'pull_request') {
$refParts = explode('/', $githubRef);
$prNumber = $refParts[2];
$this->env['CI_PULL_REQUEST'] = $prNumber;
$this->readEnv['CI_PULL_REQUEST'] = $this->env['CI_PULL_REQUEST'];
$jobId = sprintf('%s-PR-%s', $githubSha, $prNumber);
} elseif (strpos($githubRef, 'refs/tags/') !== false) {
$githubRef = str_replace('refs/tags/', '', $githubRef);
$jobId = $githubRef;
}

$this->env['CI_JOB_ID'] = $jobId;
Copy link

@OndraM OndraM Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$this->env['CI_JOB_ID'] = $jobId;
$this->env['CI_JOB_ID'] = $this->env['GITHUB_RUN_NUMBER'];

$this->env['GITHUB_RUN_NUMBER'] is now available should be used instead of the custom commit hash based value.

https://docs.github.com/en/actions/configuring-and-managing-workflows/using-environment-variables#default-environment-variables

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OndraM maybe, but seems like maintainers ignore this PR and communication with community is broken

$this->env['CI_BRANCH'] = $githubRef;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referential implementation did not send this if I remember correctly. Currently I'm using it without setting branch.

The problem is that $githubRef is not always branch so I'm not sure this is correct.

Copy link
Contributor Author

@Smolevich Smolevich Jan 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that itsn't principal moment


$this->readEnv['GITHUB_ACTIONS'] = $this->env['GITHUB_ACTIONS'];
$this->readEnv['GITHUB_REF'] = $this->env['GITHUB_REF'];
$this->readEnv['CI_NAME'] = $this->env['CI_NAME'];
$this->readEnv['CI_JOB_ID'] = $this->env['CI_JOB_ID'];
$this->readEnv['CI_BRANCH'] = $this->env['CI_BRANCH'];

return $this;
}

/**
* Fill CircleCI environment variables.
*
Expand Down
4 changes: 2 additions & 2 deletions src/Bundle/CoverallsBundle/Collector/GitInfoCollector.php
Expand Up @@ -74,9 +74,9 @@ protected function collectBranch()

foreach ($branchesResult as $result) {
if (strpos($result, '* ') === 0) {
$exploded = explode('* ', $result, 2);
preg_match('~^\* (?:\(HEAD detached at )?([\w/]+)\)?~', $result, $matches);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks for branches which have hyphens (-) and other non-word characters in them. This method incorrectly extracts my for the branch name when the full branch name is my-branch for example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Smolevich I'll fix this and send you PR, k?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My branch has hyphen and this code don't break

Copy link

@emmetog emmetog Feb 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(edit: fixed the code of the test)

Here's a test which replicates the problem (feel free to use it in this PR):

    /**
     * @test
     */
    public function shouldParseCaseInsensitive()
    {
        $branches = [
            '  TEST-123',
            '* TEST-456',
            '  TEST-789',
        ];

        $gitCommand = $this->createGitCommandStubWith($branches, $this->getHeadCommitValue, $this->getRemotesValue);

        $object = new GitInfoCollector($gitCommand);

        $result =$object->collect();

        $this->assertEquals('TEST-456', $result->getBranch());
    }

I've seen that github actions checks out the code in detached HEAD (HEAD detached at...) but when running locally it's not detached and it returns TEST instead of TEST-456.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also recovered support for hyphens
#298


return $exploded[1];
return $matches[1];
}
}

Expand Down
Expand Up @@ -72,6 +72,11 @@ public function getHelpMessage()
- APPVEYOR
- APPVEYOR_BUILD_NUMBER

For Githib Actions users:
- GITHUB_REF
- GITHUB_SHA
- GITHUB_ACTIONS
Comment on lines +76 to +78
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only COVERALLS_REPO_TOKEN is needed. The rest is set by GA.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See

, this variable was setted by Travis, but she has in error message

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I'm using it without those variables explicitly set so I'm not sure we should require users to do so.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@Smolevich Smolevich Jan 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand you, but it's linked with #275 (review), if maintainer @keradus answer on my question about required variable for GA

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@simPod i add new function requireGithubActions that can work without using COVERALLS_RUN_LOCALLY. See https://github.com/Smolevich/laravel-mongodb/commit/adb57c22b5358673945c9dc452ab7e94ee9e6a33/checks?check_suite_id=405300949

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users don't set variables like GITHUB_SHA in ci file, it part of environment for ci build. We can descriprion to README for example ci in GA


From local environment:

- COVERALLS_RUN_LOCALLY
Expand Down
14 changes: 14 additions & 0 deletions src/Bundle/CoverallsBundle/Entity/JsonFile.php
Expand Up @@ -547,6 +547,10 @@ protected function ensureJobs()
return $this;
}

if ($this->requireGithubActions()) {
return $this;
}

if ($this->isUnsupportedServiceJob()) {
return $this;
}
Expand Down Expand Up @@ -594,6 +598,16 @@ protected function requireRepoToken()
return $this->serviceName === 'travis-pro' && $this->repoToken !== null;
}

/**
* Return whether the job requires "service_number", "service_job_id" and "repo_token" (for GithubActions).
*
* @return bool
*/
protected function requireGithubActions()
{
return $this->serviceName === 'github' && $this->serviceJobId !== null && $this->repoToken !== null;
}

/**
* Return whether the job is running on unsupported service.
*
Expand Down
79 changes: 79 additions & 0 deletions tests/Bundle/CoverallsBundle/Collector/CiEnvVarsCollectorTest.php
Expand Up @@ -134,6 +134,66 @@ public function shouldCollectJenkinsEnvVars()
return $object;
}

/**
* @test
*/
public function shouldCollectGithubActionsEnvVars()
{
$serviceName = 'github';
$jobId = '3fd27e3b7c1ae6a0931d3b637b742440f5eb5011';
$gitTag = 'v0.1.1';

$env = [];
$env['COVERALLS_REPO_TOKEN'] = 'token';
$env['GITHUB_ACTIONS'] = true;
$env['GITHUB_EVENT_NAME'] = 'push';
$env['GITHUB_REF'] = 'refs/heads/master';
$env['GITHUB_SHA'] = $jobId;

$object = $this->createCiEnvVarsCollector();

$actual = $object->collect($env);

$this->assertArrayHasKey('CI_NAME', $actual);
$this->assertSame($serviceName, $actual['CI_NAME']);

$this->assertArrayHasKey('CI_JOB_ID', $actual);
$this->assertSame($jobId, $actual['CI_JOB_ID']);

$env['GITHUB_REF'] = 'refs/tags/' . $gitTag;
$actual = $object->collect($env);
$this->assertSame($gitTag, $actual['CI_JOB_ID']);

return $object;
}

/**
* @test
*/
public function shouldCollectGithubActionsEnvVarsForPullRequest()
{
$serviceName = 'github';

$env = [];
$env['COVERALLS_REPO_TOKEN'] = 'token';
$env['GITHUB_ACTIONS'] = true;
$env['GITHUB_EVENT_NAME'] = 'pull_request';
$env['GITHUB_REF'] = 'refs/pull/1/merge';
$env['GITHUB_SHA'] = '3fd27e3b7c1ae6a0931d3b637b742440f5eb5011';

$object = $this->createCiEnvVarsCollector();

$actual = $object->collect($env);

$this->assertArrayHasKey('CI_NAME', $actual);
$this->assertSame($serviceName, $actual['CI_NAME']);

$this->assertArrayHasKey('CI_JOB_ID', $actual);
$this->assertSame('3fd27e3b7c1ae6a0931d3b637b742440f5eb5011-PR-1', $actual['CI_JOB_ID']);

return $object;
}

/**
* @test
*/
Expand Down Expand Up @@ -232,6 +292,25 @@ public function shouldHaveReadEnvAfterCollectTravisCiEnvVars(CiEnvVarsCollector
$this->assertArrayHasKey('CI_NAME', $readEnv);
}

/**
* @test
* @depends shouldCollectGithubActionsEnvVars
*
* @param CiEnvVarsCollector $object
*/
public function shouldHaveReadEnvAfterCollectGithubActionsEnvVars(CiEnvVarsCollector $object)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how is it possible to use param type hint on PHP 5.5? 🤔Build is green

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like that build runs only php 7.1.2 in appveyor

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are run in 5.5 https://travis-ci.org/php-coveralls/php-coveralls/jobs/632963135 as well. Mysterious :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keradus, can we delete ci for php 5.5?

{
$readEnv = $object->getReadEnv();

$this->assertCount(6, $readEnv);
$this->assertArrayHasKey('GITHUB_REF', $readEnv);
$this->assertArrayHasKey('CI_NAME', $readEnv);
$this->assertArrayHasKey('CI_JOB_ID', $readEnv);
$this->assertArrayHasKey('GITHUB_ACTIONS', $readEnv);
$this->assertArrayHasKey('CI_BRANCH', $readEnv);
$this->assertArrayHasKey('COVERALLS_REPO_TOKEN', $readEnv);
}

/**
* @test
* @depends shouldCollectTravisProEnvVars
Expand Down
17 changes: 17 additions & 0 deletions tests/Bundle/CoverallsBundle/Collector/GitInfoCollectorTest.php
Expand Up @@ -74,6 +74,23 @@ public function shouldCollect()
$this->assertGit($git);
}

/**
* @test
*/
public function shouldCollectDetachedRef()
{
$gitCommand = $this->createGitCommandStubWith(
['* (HEAD detached at pull/1/merge)'],
$this->getHeadCommitValue,
$this->getRemotesValue
);
$object = new GitInfoCollector($gitCommand);

$git = $object->collect();

$this->assertSame('pull/1/merge', $git->getBranch());
}

// collectBranch() exception

/**
Expand Down