Skip to content
This repository was archived by the owner on Jul 18, 2023. It is now read-only.

refactor(scripts): add comments, remove duplicates, augment jest #263

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ jobs:
# paths:
# - ./node_modules
# - run:
# # TODO: Update
# # TODO: Update below - you may be able to remove one or all of the --hosts/--db/--aws options
# # depending on what scripts you have in .circleci/scripts
# name: run setup.sh
# command: |
# export $(egrep -v '^#' .docker/local.env | xargs -0) && ./.circleci/scripts/setup.sh --hosts --db --aws
Expand Down
Empty file modified .circleci/scripts.example/setup.sh
100644 → 100755
Empty file.
Empty file modified .circleci/scripts.example/setup_aws.sh
100644 → 100755
Empty file.
Empty file modified .circleci/scripts.example/setup_db.sh
100644 → 100755
Empty file.
2 changes: 2 additions & 0 deletions .circleci/scripts.example/setup_hosts.sh
100644 → 100755
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
#!/bin/bash
set -e

# Add host entries to match local docker development names.
echo "Adding service hosts records"

# These should match the services you are creating in your docker-compose file
# This is required for containers to talk to each other
Copy link
Contributor

@kkyeboah kkyeboah Nov 2, 2021

Choose a reason for hiding this comment

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

Suggested change
# This is required for containers to talk to each other
# This is required for CircleCI to talk to containers if you export environment variables to the shell that change the host of running containers from localhost to something else like `mysql` and `localstack`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you explain this a bit more? in this instance does "CircleCI" mean a server (not a container) within circle's infrastructure that talks to our specified containers?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jpetto From my understanding, when a job is triggered on Circle CI, it provisions a VM (think your local machine) and runs the containers, exposing them on localhost.

Before we run integration tests we export the environment variables that our application depends on to the shell. This means that when tests are running and using configurations from our config/index.ts file (which pulls from these env vars), what it reads are the env var definitions for the docker container. CircleCI does not know what mysql means, so we tell CircleCI that when you see mysql, please look at it as localhost or 127.0.0.1.

Now you may be asking, just as I'm asking myself now, that with projects like prospect-api that are set up to run outside the container without needing any of these env vars, why do we export anything to the shell? I think the answer is that we don't need to and CIrcleCI should just work. I will confirm this after these short messages :)

Let me know if a live chat might help a bit more here.

declare -a arr=("mysql" "localstack" "snowplow")

for i in "${arr[@]}"; do
Expand Down
16 changes: 0 additions & 16 deletions .circleci/scripts/setup.sh

This file was deleted.

29 changes: 0 additions & 29 deletions .circleci/scripts/setup_db.sh

This file was deleted.

16 changes: 0 additions & 16 deletions .circleci/scripts/setup_hosts.sh

This file was deleted.

2 changes: 2 additions & 0 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ module.exports = {
testEnvironment: 'node',
testMatch: ['**/?(*.)+(spec|integration).ts'],
testPathIgnorePatterns: ['/dist/'],
// TODO: remove the following if you don't have any localstack integration tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// TODO: remove the following if you don't have any localstack integration tests
// TODO: remove the following if you don't have any setup you would like to do before your tests run. Ex. setting a different endpoint for localstack services for integration tests

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 was under the impression that if localstack was in use, an endpoint had to be defined - e.g. http://localstack:4566. is that not the case?

Copy link
Contributor

@kkyeboah kkyeboah Nov 3, 2021

Choose a reason for hiding this comment

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

My comment is more to the point that this config allows including a setup file that is not just for setting up env vars for localstack, you can pretty much do anything in the setup file like, for example, create a tmp directory for all tests to output something to.

setupFiles: ['./jest.setup.js'],
};
4 changes: 4 additions & 0 deletions jest.setup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// your AWS_ENDPOINT values should be set in .docker/local.env
// this file is to provide a value when running tests locally outside of the container
// TODO: if you don't have any localstack integration tests, you can delete this file
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// TODO: if you don't have any localstack integration tests, you can delete this file
// TODO: if you don't have any localstack integration tests, you can delete this file
// Generally, if you don't have any setup you would like to do before tests, you can delete this entire file

process.env.AWS_ENDPOINT = process.env.AWS_ENDPOINT || 'http://localhost:4566';
4 changes: 4 additions & 0 deletions src/config/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ export default {
environment: process.env.NODE_ENV || 'development',
defaultMaxAge: 86400,
},
// TODO: if you aren't using localstack, you can get rid of the `aws` section
aws: {
localEndpoint: process.env.AWS_ENDPOINT,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
},

// TODO: Update example cache configuration below if necessary.
redis: {
primaryEndpoint: process.env.REDIS_PRIMARY_ENDPOINT || 'redis',
Expand Down