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.

Sign up for GitHub

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

Jump to bottom

Add github actions #145

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add github actions #145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da1d6cf
:construction_worker: add tests.yml
DanielGSoftware Feb 10, 2022
fccf97a
:white_check_mark: solve risky test alert
DanielGSoftware Feb 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
name: Tests

on: [push]
Copy link
Member

Choose a reason for hiding this comment

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

should this be on: [push, merge_requests] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure, but in my experience adding both will result in the tests running twice, as you can see on a test PR I made here (tests failing can be ignored, just for demonstrating).
image

Copy link
Member

Choose a reason for hiding this comment

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

that is correct. by using the merge request option it runs one set of actions on the branch and one more set of actions on the potential merge commit that would be originated if the merge request is merged.

the last part is important for the case in which your local branch is not up to date with the latest main/master branch.

this is the user case:

  1. you create a new branch starting from main, you start adding commits there, and the build is green
  2. in the mean time someone adds a bunch of commits to main (as example making other merge requests or pushing directly to the branch) and the build is also green, but the changes are not compatible with the changes you did in your branch (even if there are no git conflicts)
  3. at this point both branches have a green build, but if you merge your branch, the resulting pipeline will be broken

with the merge request builds you reduce the possibility of failure. in reality the merge request approach does not give you the guarantee that the merged result will still be green. there are mainly two popular approaches to have the build always green.

  1. run the merge request actions after pressing the merge request button, and cancel the merge if the actions build fails
  2. enforce that the merge request branch is always in sync with the latest main branch

both this approaches have advantages and downsides, but is are one of the most common approaches to ensure a green build


jobs:
test:

runs-on: ubuntu-latest
strategy:
fail-fast: true
matrix:
php: [7.2, 7.3, 7.4, 8.0, 8.1]

name: PHP ${{ matrix.php }}

steps:

- name: Checkout Code
uses: actions/checkout@v2

- name: Cache Dependencies
uses: actions/cache@v2
with:
path: ~/.composer/cache/files
key: dependencies-php-${{ matrix.php }}-composer-${{ hashFiles('composer.json') }}

- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php }}
extensions: curl, mbstring, zip, pcntl, pdo, sqlite, pdo_sqlite, iconv, mcrypt
coverage: none

- name: Install Dependencies
run: |
composer install --no-interaction
- name: Execute Tests
run: ./vendor/bin/phpunit
2 changes: 2 additions & 0 deletions tests/JmsSerializer/OTA/OTASerializationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ public function testValidation($xml, $xsd, $class)
if (@$xmlDom->schemaValidate($xsd)) {
$this->assertCount(0, $violations, 'Validation errors in ' . $xml);
}

self::assertTrue(true);
}

private static function getXmlFiles()
Expand Down