Skip to content

feat: allow developers to bypass the docker build #11

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

Merged
merged 4 commits into from
Mar 24, 2023

Conversation

stonecharioteer
Copy link
Contributor

Changes

  • This allows devs to bypass the docker build step. Now, the docker image will only be built when DOCKER_BUILD environment variable is set.`

Tests

To build the docker image before running tests:

DOCKER_BUILD=1 cargo test

To run tests normally,

cargo test

Note that you could use this with cargo check and cargo build as well, but that doesn't work really.

Issues

@stonecharioteer stonecharioteer requested a review from noot March 20, 2023 11:08
@@ -10,9 +10,12 @@ Ensure that docker is installed on the machine where the tests need to be run.

Then, run the following as usual.
```
cargo test
DOCKER_BUILD=1 cargo test
Copy link
Contributor

Choose a reason for hiding this comment

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

does the value this is set to matter, or does it just need to be set for env::var("DOCKER_BUILD").is_ok() to be true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The value doesn't matter. Just that it is set to something. So you could do DOCKER_BUILD=candy and that would work also.

Copy link
Contributor

@noot noot left a comment

Choose a reason for hiding this comment

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

looks good, can you update the github workflow to also have DOCKER_BUILD set?

@stonecharioteer stonecharioteer force-pushed the stonecharioteer/optional-docker-build branch from 538e325 to cb4676a Compare March 23, 2023 09:31
@stonecharioteer
Copy link
Contributor Author

I used act to test the action locally. Made some slight improvements to the action file itself, let's see if it works here.

* Update README to showcase how to write tests using the macro instead
* Update README to note about the ws:// prefix.
* Update github actions to use the rust action instead of setting the
  stable toolchain manually, and use `DOCKER_BUILD` flag at least once
  before the tests.
* Run multiple tests simultaneously.
@stonecharioteer stonecharioteer force-pushed the stonecharioteer/optional-docker-build branch from cb4676a to 9c979be Compare March 23, 2023 09:54
}
}
```

One can create as many of these as needed, limited only by the server resources.

**NOTE**: When using a URI for connections, ensure that you're prefixing the
Copy link
Contributor

Choose a reason for hiding this comment

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

this might be sort of confusing since dialer_uri already has ws:// in it? I'd just remove this for now lol

@stonecharioteer stonecharioteer merged commit e1e7552 into main Mar 24, 2023
@stonecharioteer stonecharioteer deleted the stonecharioteer/optional-docker-build branch March 24, 2023 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow bypassing docker build in the build.rs file for quicker builds.
2 participants