Skip to content
This repository has been archived by the owner on Nov 14, 2020. It is now read-only.

Travis: Execute acceptance tests on dockerized RMQ. #47

Merged
merged 1 commit into from
Jan 7, 2020
Merged

Conversation

cyrilgdn
Copy link
Contributor

@cyrilgdn cyrilgdn commented Jan 6, 2020

No description provided.

Copy link
Contributor

@multani multani left a comment

Choose a reason for hiding this comment

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

Thanks @cyrilgdn for enabling this, it would show that the test I wrote in #45 (without the fix) would have failed so 👍 for that!

.travis.yml Outdated
@@ -17,13 +28,18 @@ script:
- make test
- make vet
- make website-test
- docker-compose -f $(pwd)/scripts/docker-compose.yml up -d
- $(pwd)/scripts/wait-rabbitmq-docker.sh $(pwd)/scripts/docker-compose.yml
Copy link
Contributor

Choose a reason for hiding this comment

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

You are passing the docker-compose file but it's not really used inside the script, I guess you can remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed 👍 , a bit copy/pasted from another provider where it was used 😅

rabbitmq:
image: rabbitmq:${RABBITMQ_VERSION:-3.8}-management-alpine
ports:
- 15672:15672
Copy link
Contributor

Choose a reason for hiding this comment

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

To make things more explicit, could you also pass the environment variables that you defined for the username, password in here?

env:
  RABBITMQ_DEFAULT_USER: ${RABBITMQ_USERNAME:guest}
  RABBITMQ_DEFAULT_PASS: ${RABBITMQ_PASSWORD:guest}

scripts/wait-rabbitmq-docker.sh Outdated Show resolved Hide resolved
@cyrilgdn
Copy link
Contributor Author

cyrilgdn commented Jan 7, 2020

Thanks @multani for your review !

I updated the PR based on your remarks

Copy link
Contributor

@multani multani 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 to me, let me know when it's merged and I can rebase #45 on top of it 👍

@cyrilgdn cyrilgdn merged commit df068c9 into master Jan 7, 2020
@cyrilgdn cyrilgdn deleted the travis branch January 7, 2020 09:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants