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

Configuration to run tests in Github Actions #271

Merged
merged 2 commits into from
Mar 2, 2022

Conversation

VadimSchmitz
Copy link
Contributor

@VadimSchmitz VadimSchmitz commented Mar 2, 2022

In this pullrequest a docker-compose file has been added for the testing of code on a container.
The github action workflow has been configured and passes the tests.

adds docker compoer file that will be used for the conversion from
travisci to github actions
@VadimSchmitz VadimSchmitz changed the title Configuration to run tests in Configuration to run tests in Github Actions Mar 2, 2022
@VadimSchmitz VadimSchmitz requested a review from MKodde March 2, 2022 12:07
Copy link
Member

@MKodde MKodde left a comment

Choose a reason for hiding this comment

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

Awesome work Vadim.

Feel free to merge this and fix the two pointers on your next PR. That way you don't have to rebase your current work onto these changes.

Edit:
Looking at your commit messages, I have some critique:

  1. The title of your first commit Add docker-composer is not doing it for me. I can deduct that you are adding something related to docker-composer. But consider making it more descriptive like: Create the test-integration docker-compose.yml file or something along those lines. Short is not always better (but it often is ;) )
  2. Try to re-read your commits and do some self-reviewing. That way you can catch some typos, maybe further sharpen a title, or even come to the conclusion that your commit needs to be squashed or otherwise modified.

You have come a long way in 7 days! These pointers are not to bring you down but just to raise the bar a little bit 👍

## ant should be changed to a composer script somehow
- ant
- composer test
on: [ pull_request, push ]
Copy link
Member

Choose a reason for hiding this comment

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

This will do the job just fine. But maybe to preserve some GHA clock-ticks you could limit the branches on which a test-integration run is triggered.

Before we had:

on:
    pull_request:
    push:
        branches: [master, develop]

That might be slightly better from running on every push.

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 reverted it back to how it was.

run: composer test
- name: Output log files on failure
if: failure()
run: cd ci/docker && docker-compose exec -T php-fpm.stepup.example.com cat var/log/webtest.log
Copy link
Member

Choose a reason for hiding this comment

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

Have you tested an error situation? I highly doubt this line will work :)

Feel free to fix this on your next PR 💪

@VadimSchmitz VadimSchmitz force-pushed the feature/travis-to-gha branch 12 times, most recently from e646a1e to e6466e0 Compare March 2, 2022 13:46
The following test-integration workflow will start up the
container using the docker-composer file. afterwards it will do some
installation and then run the tests.
Also fixed typo in file name of docker-compose
@VadimSchmitz VadimSchmitz force-pushed the feature/travis-to-gha branch from e6466e0 to b35d5cf Compare March 2, 2022 13:48
run: composer test
- name: Output log files on failure
if: failure()
run: cd ci/docker && docker-compose exec -T ra-test cat var/log/error.log
Copy link
Member

Choose a reason for hiding this comment

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

Okay with me to leave this like this for now.

@VadimSchmitz VadimSchmitz merged commit 603936b into develop Mar 2, 2022
@VadimSchmitz VadimSchmitz deleted the feature/travis-to-gha branch March 2, 2022 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants