Skip to content
This repository was archived by the owner on Mar 23, 2026. It is now read-only.

Split out bootstrap tests for CircleCI#8768

Merged
simonrw merged 8 commits into
masterfrom
bootstrap-ci
Aug 29, 2023
Merged

Split out bootstrap tests for CircleCI#8768
simonrw merged 8 commits into
masterfrom
bootstrap-ci

Conversation

@simonrw
Copy link
Copy Markdown
Contributor

@simonrw simonrw commented Jul 28, 2023

Move the bootstrap tests into their own location and run them in CircleCI

  • the tests don't need to run against multiple python versions
  • the tests are now run on CircleCI concurrently with the integration tests

@simonrw simonrw added the semver: patch Non-breaking changes which can be included in patch releases label Jul 28, 2023
@simonrw simonrw self-assigned this Jul 28, 2023
@coveralls
Copy link
Copy Markdown

coveralls commented Jul 28, 2023

Coverage Status

coverage: 80.71% (-0.01%) from 80.723% when pulling b195764 on bootstrap-ci into cae0583 on master.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 28, 2023

LocalStack Community integration with Pro

       2 files         2 suites   1h 23m 16s ⏱️
2 149 tests 1 671 ✔️ 478 💤 0
2 150 runs  1 671 ✔️ 479 💤 0

Results for commit b195764.

♻️ This comment has been updated with latest results.

@simonrw simonrw marked this pull request as ready for review August 18, 2023 16:56
@simonrw simonrw removed request for dfangl and thrau August 18, 2023 16:56
@simonrw simonrw requested a review from dfangl August 24, 2023 09:52
Copy link
Copy Markdown
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

We will extend the bootstrap tests quite a lot in the future (we have plenty of use-cases for these lifecycle tests), so this is nice ground-work preparing the tests to come.
I only found a small thing about the creation of the host directories, afterwards we're good to go! 🚀

Comment thread localstack/utils/bootstrap.py Outdated
'LocalStack container named "%s" is already running' % self.container.name
)

config.dirs.mkdirs()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess this line was added because some tests failed with the new bootstrap tests (because it seems that they were depending on the CLI tests). But imho this doesn't belong here, because the LocalstackContainer class does not define any volume mounts for the config dirs on its own.

The mkdirs creates the directories depending on where it's executed. If it's executed in Docker, it will create the directories for within the Docker container. If it's executed on the host, it will create the directories for the host system.

The creation of the directories at runtime - within the container - is invoked in start_infra:

config.dirs.mkdirs()

In our case here, since it's executed before the container is started, it will create the host directories.
Since these mounts are not configured by the LocalstackContainer class, but by a using component, it is also in the responsibility of this using compontent to create the directories.
For starting with the CLI, the directories are created in prepare_docker_start:

def prepare_docker_start():

This is a utility function used in combination with bootstrap.configure_container, which configures the volumes.

TL/DR: Creating the dirs is in the responsibility of the users of this class. Please move this line to the caller(s).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That makes sense, thanks.

@simonrw simonrw requested a review from alexrashed August 29, 2023 10:08
Copy link
Copy Markdown
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the comment! Looking good! 🚀 :shipit:

@simonrw simonrw merged commit 1c7e3ac into master Aug 29, 2023
@alexrashed alexrashed deleted the bootstrap-ci branch August 29, 2023 14:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants