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

added initial draft github action workflow #1299

Closed
wants to merge 5 commits into from
Closed

added initial draft github action workflow #1299

wants to merge 5 commits into from

Conversation

auvipy
Copy link
Member

@auvipy auvipy commented Feb 25, 2021

initial experiments

@auvipy
Copy link
Member Author

auvipy commented Feb 25, 2021

Error: Container operations are only supported on Linux runners -- so integration tests with docker need Linux only

strategy:
fail-fast: false
matrix:
os: [ubuntu-20.04, macos-latest]
Copy link
Member Author

Choose a reason for hiding this comment

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

macos & windows can run unit tests and container based integration can run on llinux only, so separate jobs should be created

Copy link
Member

Choose a reason for hiding this comment

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

See this for a solution on MacOS:
actions/runner-images#1143 (comment)

@auvipy
Copy link
Member Author

auvipy commented Feb 28, 2021

related celery/py-amqp#358

if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
- name: Run Unit test with tox-docker
run: |
PYTHONPATH=. pytest -xv --cov=celery --cov-report=xml --cov-report term t/unit
Copy link
Member

Choose a reason for hiding this comment

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

Please just use tox since that's what everyone uses when cloning the project.
We need to ensure it functions perfectly.

@auvipy
Copy link
Member Author

auvipy commented Mar 5, 2021

I am a little bit sick. I saw PR of @matusvalo in pyamqp, which I can take as a suggested way. @matusvalo you can push on CI branch as well if got time.

@matusvalo
Copy link
Member

@auvipy I was checking your branch but your approach is totally different from my PR so I decided to create separate new PR of it. (there is no benefit integrating it in your PR since I would overwrite whole file). Though, your PR contains some great stuff (like coverage, cache etc). Maybe after accepting basic PR of CI you could create PR for adding them to it? (I would like to have simple running CI ASAP, since we introduced some issues/failing unit tests due not having CI running)

@auvipy
Copy link
Member Author

auvipy commented Mar 7, 2021

@auvipy I was checking your branch but your approach is totally different from my PR so I decided to create separate new PR of it. (there is no benefit integrating it in your PR since I would overwrite whole file). Though, your PR contains some great stuff (like coverage, cache etc). Maybe after accepting basic PR of CI you could create PR for adding them to it? (I would like to have simple running CI ASAP, since we introduced some issues/failing unit tests due not having CI running)

OK

@thedrow
Copy link
Member

thedrow commented Mar 7, 2021

Should we close this then?

@matusvalo
Copy link
Member

I think yes. But we can use some parts of this PR which are missing in current CI (cache, covetage etc)

@auvipy
Copy link
Member Author

auvipy commented Mar 8, 2021

i will improve this PR

@auvipy auvipy closed this Mar 12, 2023
@auvipy auvipy deleted the ci branch March 12, 2023 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants