-
Notifications
You must be signed in to change notification settings - Fork 505
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
\#1500 - prototype for using testcontainers-go for dependencies #1544
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
name: Go | ||
on: | ||
- push | ||
- pull_request | ||
jobs: | ||
|
||
build: | ||
name: Build | ||
runs-on: ubuntu-latest | ||
steps: | ||
|
||
- name: Set up Go 1.13 | ||
uses: actions/setup-go@v1 | ||
with: | ||
go-version: 1.13 | ||
id: go | ||
|
||
- name: Check out code into the Go module directory | ||
uses: actions/checkout@v1 | ||
|
||
- name: Test | ||
run: ATHENS_USE_TEST_CONTAINERS=1 go test -v ./... |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,20 @@ | ||
image: Visual Studio 2019 | ||
build: off | ||
|
||
clone_folder: c:\gopath\src\github.com\gomods\athens | ||
|
||
image: Previous Visual Studio 2019 | ||
|
||
environment: | ||
GOPATH: c:\gopath | ||
GO111MODULE: on | ||
GOPROXY: https://proxy.golang.org | ||
SKIP_UNTIL_113: true | ||
ATHENS_USE_TEST_CONTAINERS: 1 | ||
|
||
stack: go 1.13 | ||
|
||
stack: go 1.14 | ||
install: | ||
- choco upgrade docker-desktop | ||
|
||
test_script: | ||
- go version | ||
- go test ./... | ||
|
||
- go test -v ./... |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,15 +22,12 @@ services: | |
args: | ||
GOLANG_VERSION: 1.14 | ||
command: ["./scripts/test_unit.sh"] | ||
volumes: | ||
- /var/run/docker.sock:/var/run/docker.sock # test this on windows, it might be different | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @arschles do you have a Windows machine? Could you ensure that the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe it is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can try, but let's try to get testcontainers-less appveyor tests back 😄 |
||
environment: | ||
- GO_ENV=test | ||
- ATHENS_MINIO_ENDPOINT=minio:9000 | ||
- ATHENS_MONGO_STORAGE_URL=mongodb://mongo:27017 | ||
- TIMEOUT=20 # in case the mongo dependency takes longer to start up | ||
- ATHENS_STORAGE_TYPE=mongo | ||
depends_on: | ||
- mongo | ||
- minio | ||
teste2e: | ||
build: | ||
context: . | ||
|
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
// +build !unit | ||
|
||
package stash | ||
|
||
import ( | ||
|
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.
@ClaytonNorthey92 we still need to run at least unit tests on windows. One idea would be to add an env var to turn off the testcontainers tests and set it in appveyor but not in GH actions. What do you think?
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.
so the issue here came from trying to start the reaper containers on windows (the containers that run and ensure that the containers we start get killed at the end of the tests), but you can optionally skip the reaper containers.
I think that since, according to appveyor's documentation, they don't preserve any VMs between builds, we can safely skip the reaper in appveyor tests. So I think we should add that env variable like
TESTCONTAINERS_SKIP_REAPER
, if that is set to1
then we skip the reaper builds and we should be okay (I think)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.
I am testing this theory out now...I accidentally got a build stuck debugging...so I am going to wait for that to timeout and fail to keep on testing
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.
hey @arschles how do you feel about putting integration tests into their own package? the using a build tag to ignore them by default, then you can pass in the tag to enable them? that should be able to be done just from the code and the config files and wouldn't require a change to the environment
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.
Fine with me RE the different package. I'd rather have them enabled by default and pass a tag to disable them. Are you cool with that?
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.
yeah that makes sense to me, just need a way to cleanly include/not include them
I can work on this
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.
That's great! We had an idea in #1500 to not turn off the tests completely, but just to turn off the testcontainers logic based on a tag. I laid it out here
What do you think about that?
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.
just left a comment