-
Notifications
You must be signed in to change notification settings - Fork 3
Fix tests and run tests on ci #61
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
base: localstack
Are you sure you want to change the base?
Conversation
d29d9d0 to
64f8259
Compare
.github/workflows/build.yml
Outdated
| test: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: '1.25' | ||
|
|
||
| - name: Run tests | ||
| run: make tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not rather add (or perhaps even replace) the localstack branch inside the Run Integration Tests job?
lambda-runtime-init/.github/workflows/integ-tests.yml
Lines 3 to 6 in 5b81e4c
| on: | |
| pull_request: | |
| branches: | |
| - develop |
Seems there is a job there that will ensure the Go tests are run prior to running the int. tests:
lambda-runtime-init/.github/workflows/integ-tests.yml
Lines 8 to 16 in 5b81e4c
| jobs: | |
| go-tests: | |
| runs-on: ubuntu-latest | |
| environment: | |
| name: integ-tests | |
| steps: | |
| - uses: actions/checkout@v4 | |
| - name: run go tests | |
| run: make tests-with-docker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing this out, I had missed this integ-tests.yml file!
What I did is delete it, since:
- most jobs it contains are failing at the moment
- the job I added is about unit tests, whereas this workflow is called integ-tests
- I think it makes sense to always run the unit tests before attempting to build
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha. Can we keep the integration tests yaml, and just ensure it's skipped/never runs? I think in future, we should be running integration tests with upstream LocalStack IMO.
For example, we can build the go binary, upload it as a workflow artifact, and trigger some upstream LocalStack workflow that ensures we run the Lambda suite with this binary.
gregfurman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! The CI on this repo needs some tlc, so thanks for taking the first step 🙂
I think we should invest some time in better integrating the RIE's CI with upstream LocalStack, but we can tackle this at some other point.
Uh oh!
There was an error while loading. Please reload this page.