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

run makefile tests for every sample in a separate CI workflow #247

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

run makefile tests for every sample in a separate CI workflow #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7a84246
take a stab
HarshCasper Jun 3, 2024
9925caa
run actual tests now
HarshCasper Jun 3, 2024
07a281c
set mindepth to 2:
HarshCasper Jun 3, 2024
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
110 changes: 55 additions & 55 deletions .github/workflows/makefile.yml
View file
Open in desktop
Copy link

Choose a reason for hiding this comment

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

if we are commenting the whole file out, why not just remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, that would be removed once I have an explicit approval that this is the way to go forward 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, with git being used properly and everywhere, there is never a need for commented out code. You can always jump back, and commented code can't make it into code reviews or even in the mainline (where it just causes confusion and maintenance overhead - also for example like these comments here 😛).

Original file line number Diff line number Diff line change
@@ -1,64 +1,64 @@
name: Makefile CI
# name: Makefile CI

on:
push:
branches: [ "master" ]
pull_request:
branches: [ "master" ]
schedule:
- cron: '0 13 * * 1'
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3
# on:
# push:
# branches: [ "master" ]
# pull_request:
# branches: [ "master" ]
# schedule:
# - cron: '0 13 * * 1'
# jobs:
# test:
# runs-on: ubuntu-latest
# steps:
# - name: Checkout
# uses: actions/checkout@v3

- name: Setup Nodejs
uses: actions/setup-node@v3
with:
node-version: 14
# - name: Setup Nodejs
# uses: actions/setup-node@v3
# with:
# node-version: 14

# see https://github.com/localstack/localstack/pull/6831
# remove once no longer needed
- name: Fix pyOpenSSL version
run: pip install --upgrade pyOpenSSL
# # see https://github.com/localstack/localstack/pull/6831
# # remove once no longer needed
# - name: Fix pyOpenSSL version
# run: pip install --upgrade pyOpenSSL

- name: Install LocalStack
run: pip install localstack awscli-local[ver1]
# - name: Install LocalStack
# run: pip install localstack awscli-local[ver1]

- name: Install Dependencies
run: |
pip install virtualenv
pip install --upgrade pyopenssl
npm install -g serverless
# - name: Install Dependencies
# run: |
# pip install virtualenv
# pip install --upgrade pyopenssl
# npm install -g serverless

- name: Setup config
run: |
echo "Configuring git for codecommit sample"
git config --global user.email "[email protected]"
git config --global user.name "Localstack Pro-Samples"
# - name: Setup config
# run: |
# echo "Configuring git for codecommit sample"
# git config --global user.email "[email protected]"
# git config --global user.name "Localstack Pro-Samples"

- name: Pull the latest docker image
run: docker pull localstack/localstack-pro
# - name: Pull the latest docker image
# run: docker pull localstack/localstack-pro

- name: Execute tests
env:
LOCALSTACK_API_KEY: ${{ secrets.TEST_LOCALSTACK_API_KEY }}
DNS_ADDRESS: 127.0.0.1
DEBUG: 1
timeout-minutes: 200
run: make test-ci-all
# - name: Execute tests
# env:
# LOCALSTACK_API_KEY: ${{ secrets.TEST_LOCALSTACK_API_KEY }}
# DNS_ADDRESS: 127.0.0.1
# DEBUG: 1
# timeout-minutes: 200
# run: make test-ci-all

- name: Send a Slack notification
if: failure() || github.event_name != 'pull_request'
uses: ravsamhq/notify-slack-action@v2
with:
status: ${{ job.status }}
token: ${{ secrets.GITHUB_TOKEN }}
notification_title: "{workflow} has {status_message}"
message_format: "{emoji} *{workflow}* {status_message} in <{repo_url}|{repo}>"
footer: "Linked Repo <{repo_url}|{repo}> | <{run_url}|View Workflow run>"
notify_when: "failure"
env:
SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }}
# - name: Send a Slack notification
# if: failure() || github.event_name != 'pull_request'
# uses: ravsamhq/notify-slack-action@v2
# with:
# status: ${{ job.status }}
# token: ${{ secrets.GITHUB_TOKEN }}
# notification_title: "{workflow} has {status_message}"
# message_format: "{emoji} *{workflow}* {status_message} in <{repo_url}|{repo}>"
# footer: "Linked Repo <{repo_url}|{repo}> | <{run_url}|View Workflow run>"
# notify_when: "failure"
# env:
# SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }}
54 changes: 54 additions & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
name: "Run Tests for all samples"

on:
push:
branches: [ "master" ]
pull_request:
branches: [ "master" ]

jobs:
prepare_list:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- id: set-matrix
run: |
echo "matrix=[$(find . -mindepth 2 -type f -name 'Makefile' ! -path './sample-archive/*' | sed 's|/Makefile||' | awk '{printf "\"%s\",", $1}' | sed 's/,$//')]" >> $GITHUB_OUTPUT
- run: echo "matrix=[$(find . -mindepth 2 -type f -name 'Makefile' ! -path './sample-archive/*' | sed 's|/Makefile||' | awk '{printf "\"%s\",", $1}' | sed 's/,$//')]"
Comment on lines +10 to +17
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a pretty dangerous approach, because it does not limit the amount of parallel runners at all. GitHub organizations have a limit of 20 concurrent Linux runners for free in public repos: https://docs.github.com/en/actions/learn-github-actions/usage-limits-billing-and-administration
This means that it would basically block all GitHub action executions for the time it takes to execute the pipeline for the whole localstack-samples org.
Since these samples are executed somewhat fast, it can be a conscious decision to leave it like this, but I would propose to at least limit the amount of concurrent runners to less than 20 (maybe 5-10) to allow other pipelines in the org to be scheduled at the same time.
You can implement that super-easily with the max-parallel setting in the matrix config: https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs#defining-the-maximum-number-of-concurrent-jobs


outputs:
matrix: ${{ steps.set-matrix.outputs.matrix }}

runtest:
name: Run Dependency Test
runs-on: ubuntu-latest
needs: prepare_list
strategy:
fail-fast: false
matrix:
directory: ${{ fromJson(needs.prepare_list.outputs.matrix) }}

steps:
- uses: actions/checkout@v4
- name: Setup Nodejs
uses: actions/setup-node@v3
with:
node-version: 18
- name: Install LocalStack
run: pip install localstack awscli-local[ver1] virtualenv
- name: Setup config
run: |
echo "Configuring git for codecommit sample"
git config --global user.email "[email protected]"
git config --global user.name "Localstack Pro-Samples"
- name: Pull the latest docker image
run: docker pull localstack/localstack-pro
- name: Execute a simple test
timeout-minutes: 10
run: |
cd ${{ matrix.directory }}
make test-ci
env:
LOCALSTACK_API_KEY: ${{ secrets.TEST_LOCALSTACK_API_KEY }}
DEBUG: 1
DNS_ADDRESS: 127.0.0.1
Loading