feat: add integration tests #69

vincentsarago · 2023-09-05T13:22:43Z

closes #13

second take at #67

.github/workflows/deploy.yaml

This reverts commit aab0acb.

emileten · 2023-09-11T08:58:42Z

Anybody would like to take another look ? The action is running fine (I ran them manually to remove any bug). Here is a gist of what's happening :

Every time a Distribute workflow is going to complete successfully (aka every time we release)
The Deploy & Test Deployment workflow I define here picks up this release
Deploys the default https://github.com/developmentseed/eoapi-template in the DevSeed AWS account
Runs integration tests I define here
Always tears down the stack at the end.

It takes a while, almost an hour from scratch :

CDK synth ~ 6 minutes
Deployment from scratch ~ 16 minutes
Tests ~ 1 min
Stack destruction ~ 30 minutes.

For another time I'd like to figure out how to cache all the lambda runtime builds but it's a rabbit hole....

emileten · 2023-09-13T02:42:48Z

@vincentsarago @sharkinsspatial, following-up on a discussion I had with Vincent on Slack.

There is a concern regarding the fact that we are going to run the `deploy and test` workflow using the `eoapi-template` IaC repo.

What's the concern ? Let's assume that I submit a PR to eoapi-cdk with a breaking change, and I don't correspondingly modify the eoapi-template repo IaC. If I merge this PR, a release happens, which triggers a deployment using the new release wrapped into the non-updated eoapi-template code. This deployment is going to fail. It would not have failed, had I correspondingly updated eoapi-template.

This means that for any PR that triggers a release, we need to make sure we update eoapi-template if needed. This 'makes sense', because in this framework, eoapi-template became part of the test suite, and any test suite must be updated if breaking changes are merged.

Pros

eoapi-template, which was meant to be an official example, is tested and up to date.
we don't have to write wrapper IaC just for the purpose of tests, instead we reuse the official example. I am worried about writing IaC just for the tests and ending up with a lot of IaC to manage (eoapi-template, test IaC, the constructs...).

Cons

eoapi-template has to be modified often.

To this, I answer "yes but not that much often, because we have to update it only for breaking change releases. I would also answer that we do want to update eoapi-template often anyways because it's the official example"

It's hard to remember to update it because it's in a separate repo.

To this, I answer "we'd have to update any IaC we write for the purposes of the deploy-test workflow, whether it's eoAPI template or not. If the difficulty is that it's hard to remember because it's in a separate repo, I suggest that we move eoapi-template to be within eoapi-cdk."

vincentsarago · 2023-09-13T20:21:59Z

eoapi-template has to be modified often.

To this, I answer "yes but not that much often, because we have to update it only for breaking change releases. I would also answer that we do want to update eoapi-template often anyways because it's the official example"

eoapi-template will have to be modified on every release because we are using direct version pinning in eoapi-template (https://github.com/developmentseed/eoapi-template/blob/main/requirements.txt#L3)

I personally having circular dependency with external repo is dangerous is will required more work (e.g if someone change something in eoapi-cdk but has no idea what is eoapi-template is, they might open a PR and wait for someone to review, which mean that the reviewer might have to review two PR in two different repo.

I do agree that eoapi-template should be the official demo application but I do not agree that we have to keep 1:1 with the versions.

I suggest that we move eoapi-template to be within eoapi-cdk."

Because the purpose of eoapi-template is to be a repository template, I think we should could it separate. IMO it will be fine to just copy/paste what application we have in eoapi-cdk test suite to eoapi-template

emileten · 2023-09-15T09:05:26Z

Because the purpose of eoapi-template is to be a repository template, I think we should could it separate. IMO it will be > fine to just copy/paste what application we have in eoapi-cdk test suite to eoapi-template

👍

…t we dont depend on an external repository. Modify accordingly the tests folder, with two subfolders (tests and cdk code) and the top folder renamed to integration_tests

emileten · 2023-09-21T08:52:46Z

@vincentsarago made the changes, the tests here use their own wrapper CDK code. Looks like I adressed your main requests, anything else ? Once the action succeeded I will turn off the on: push and leave only the "on-release" trigger and manual option.

vincentsarago · 2023-09-25T13:16:36Z

.github/workflows/deploy.yaml

+        python -m venv .deployment_venv
+        source .deployment_venv/bin/activate
+        pip install -r requirements.txt
+        pip install --upgrade eoapi-cdk # make sure to take the latest version of eoapi-cdk since we're integration-testing the newest release. 


Are we testing the latest available version on pypi or the one in this repo?

It might be dangerous to test the version on pypi because there could be a lag from the push to the publication

Ah nice catch, I hadn't thought of the lag possibility...

However, I'd have to compile the typescript constructs locally to have access to the python modules.

How about setting up this workflow so that it waits (with a timeout) for the latest pypi to match for the latest github tag?

How about setting up this workflow so that it waits (with a timeout) for the latest pypi to match for the latest github tag?

Sure, but then what will it means if the tests fails?

Ah 🤦‍♂️ doesn't make much sense indeed. We should be doing

build -> integration test -> release

lets see if @sharkinsspatial or @alukach have opinions

Could be missing something from the CI logic, but one way to do it could be to add a job named integration-test here that reuses the package job's output. We then add integration-test as a dependency to both subsequent distribute-* jobs so that we're sure releases happen only if integration-tests ran.

FYI, to illustrate the idea, I pushed another commit (can always revert). See 4ff82c2

@emileten I would recommend relying on the local python modules from the output of the TS build from the package output as you outlined in 4ff82c2.

vincentsarago · 2023-09-25T13:18:03Z

Thanks @emileten
I'm wondering what's the purpose of doing integration test after pushing packages on npm and pypi. IMO we should do integration tests and then publish.

…ease

emileten · 2024-02-20T13:57:56Z

Making progress on this, now I am trying to figure out why I get a TimeOutError from httpx in the bootstrapper.

Because this bootstrapper runs fine on my local machine, my guess is that the bootstrapper lambda doesn't have access to the internet because of accidental configuration changes I made ; in particular, I removed allowPublicSubnet: true from the lambda creation parameters, as AWS explicitly says the lambda does not have access to the internet with this parameter.

Doesn't seem to be solving the problem, though.

Edit : oh I know what's going on - I was placing things in private subnets. I'll simplify the setup in the test to mimic https://github.com/developmentseed/eoAPI/tree/main/infrastructure/aws/cdk.

…flows

…c subnet

integration_tests/cdk/requirements.txt

… trigger to check the action works

…assets into another directory to avoid conflicts

lib/database/bootstrapper_runtime/handler.py

lib/database/index.ts

# [7.0.0](v6.1.0...v7.0.0) (2024-02-22) ### Features * add integration tests ([#69](#69)) ([17eec16](17eec16)) ### BREAKING CHANGES * clients need to provide aws_lambda.AssetCode to configure their apps. Solely the python application and the requirements.txt file is not supported anymore. * fix a couple bugs found in the first changes * avoid maintaining custom interfaces for configurable lambda properties. Allow the user to provide anything and let the CDK method raise error and overwrite values defined within our construct. Make this clear in the documentation * expose bootstrapper props in pgstacdatabase construct constructor * merge database and boostrapper files to solve casting bug * bump and harmonize pypgstac to 0.7.10 across apps * bump cachetools * some changes to allow for less security * bump python to 3.11 * change base image for bootstrapper to use python 311 * fix linting issues * move integration tests to step before release, improve naming of workflows * lint * fix moto requirement * test to fix deployment : try adding s3 endpoint and force allow public subnet * lint and make lambda functions more configurable * moving deploy to a separate workflow * remove useless dependencies in deployment tests, turn on pull request trigger to check the action works * when tearing down the infrastructure, synthesize the cloud formation assets into another directory to avoid conflicts * update readmes and revive the artifact download in python distribution

feat: add integration tests

3537795

vincentsarago commented Sep 5, 2023

View reviewed changes

.github/workflows/deploy.yaml Outdated Show resolved Hide resolved

vincentsarago commented Sep 5, 2023

View reviewed changes

.github/workflows/deploy.yaml Outdated Show resolved Hide resolved

emileten added 3 commits September 6, 2023 18:12

add eoapi-cdk upgrade command and pip cache

f1c1d1f

test commit, to revert

aab0acb

Revert "test commit, to revert"

d9749de

This reverts commit aab0acb.

emileten force-pushed the feat/add-integration-tests branch 5 times, most recently from 4494c2e to b8c14a8 Compare September 11, 2023 07:17

update workflow

1300c4e

emileten force-pushed the feat/add-integration-tests branch from b8c14a8 to 1300c4e Compare September 11, 2023 07:42

update workflow to run when release happens, move tests here

c47f6d6

emileten requested review from sharkinsspatial and alukach September 11, 2023 08:52

emileten added 2 commits September 21, 2023 17:13

instead of cloning the eoapi-template repo, copy the code here so tha…

a63c909

…t we dont depend on an external repository. Modify accordingly the tests folder, with two subfolders (tests and cdk code) and the top folder renamed to integration_tests

trailing wsp

6ebc271

emileten force-pushed the feat/add-integration-tests branch from 981ad75 to b2ca50d Compare September 21, 2023 08:19

run action with push to try

80f03ba

emileten force-pushed the feat/add-integration-tests branch from b2ca50d to 80f03ba Compare September 21, 2023 08:24

vincentsarago commented Sep 25, 2023

View reviewed changes

move the trigger of integration tests to distribute, right before rel…

4ff82c2

…ease

emileten force-pushed the feat/add-integration-tests branch from 701d85f to 4ff82c2 Compare September 27, 2023 01:07

emileten force-pushed the feat/add-integration-tests branch 2 times, most recently from 31a6845 to 01816fe Compare February 20, 2024 13:26

move integration tests to step before release, improve naming of work…

64e67b7

…flows

emileten force-pushed the feat/add-integration-tests branch from 01816fe to 64e67b7 Compare February 20, 2024 15:10

vincentsarago and others added 4 commits February 20, 2024 10:50

lint

c7904ef

fix moto requirement

92b4bf9

test to fix deployment : try adding s3 endpoint and force allow publi…

97b5a32

…c subnet

lint and make lambda functions more configurable

4ede0a5

vincentsarago commented Feb 21, 2024

View reviewed changes

integration_tests/cdk/requirements.txt Show resolved Hide resolved

vincentsarago commented Feb 21, 2024

View reviewed changes

integration_tests/cdk/requirements.txt Outdated Show resolved Hide resolved

vincentsarago commented Feb 21, 2024

View reviewed changes

integration_tests/cdk/requirements.txt Outdated Show resolved Hide resolved

moving deploy to a separate workflow

b4de035

emileten force-pushed the feat/add-integration-tests branch 3 times, most recently from 9bd845b to db7662f Compare February 21, 2024 16:10

remove useless dependencies in deployment tests, turn on pull request…

92cec0d

… trigger to check the action works

emileten force-pushed the feat/add-integration-tests branch from db7662f to 92cec0d Compare February 21, 2024 16:16

when tearing down the infrastructure, synthesize the cloud formation …

b4fe0f6

…assets into another directory to avoid conflicts

emileten reviewed Feb 21, 2024

View reviewed changes

lib/database/bootstrapper_runtime/handler.py Show resolved Hide resolved

emileten reviewed Feb 21, 2024

View reviewed changes

lib/database/index.ts Show resolved Hide resolved

update readmes and revive the artifact download in python distribution

b08e8b3

emileten added this pull request to the merge queue Feb 21, 2024

Merged via the queue into main with commit 17eec16 Feb 21, 2024
3 checks passed

emileten deleted the feat/add-integration-tests branch February 21, 2024 16:40

emileten restored the feat/add-integration-tests branch February 21, 2024 19:53

This was referenced Feb 22, 2024

Clarify github actions workflows naming #32

Closed

chore: make deployment workflow manual and add a reminder in pr template #95

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add integration tests #69

feat: add integration tests #69

vincentsarago commented Sep 5, 2023

emileten commented Sep 11, 2023

emileten commented Sep 13, 2023

vincentsarago commented Sep 13, 2023

emileten commented Sep 15, 2023

emileten commented Sep 21, 2023

vincentsarago Sep 25, 2023

emileten Sep 25, 2023

vincentsarago Sep 25, 2023

emileten Sep 25, 2023

vincentsarago Sep 25, 2023

emileten Sep 25, 2023

emileten Sep 27, 2023 •

edited

Loading

sharkinsspatial Oct 9, 2023

vincentsarago commented Sep 25, 2023

emileten commented Feb 20, 2024 •

edited

Loading

feat: add integration tests #69

feat: add integration tests #69

Conversation

vincentsarago commented Sep 5, 2023

emileten commented Sep 11, 2023

emileten commented Sep 13, 2023

There is a concern regarding the fact that we are going to run the deploy and test workflow using the eoapi-template IaC repo.

Pros

Cons

vincentsarago commented Sep 13, 2023

emileten commented Sep 15, 2023

emileten commented Sep 21, 2023

vincentsarago Sep 25, 2023

Choose a reason for hiding this comment

emileten Sep 25, 2023

Choose a reason for hiding this comment

vincentsarago Sep 25, 2023

Choose a reason for hiding this comment

emileten Sep 25, 2023

Choose a reason for hiding this comment

vincentsarago Sep 25, 2023

Choose a reason for hiding this comment

emileten Sep 25, 2023

Choose a reason for hiding this comment

emileten Sep 27, 2023 • edited Loading

Choose a reason for hiding this comment

sharkinsspatial Oct 9, 2023

Choose a reason for hiding this comment

vincentsarago commented Sep 25, 2023

emileten commented Feb 20, 2024 • edited Loading

There is a concern regarding the fact that we are going to run the `deploy and test` workflow using the `eoapi-template` IaC repo.

emileten Sep 27, 2023 •

edited

Loading

emileten commented Feb 20, 2024 •

edited

Loading