Skip to content
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

Use testcontainers for test, instead of local zerotier-one #10

Merged
merged 6 commits into from
Apr 11, 2024

Conversation

laduke
Copy link
Contributor

@laduke laduke commented Apr 11, 2024

Before, we assumed that the host/dev-env had a zerotier-one service running.

Now we can start a container and use the zerotier-one in there, and then throw it away.
No messing up your local service. No having to switch its version around. Should work for anyone with a docker runtime.

@laduke laduke force-pushed the laduke/testcontainers branch 2 times, most recently from ba733e8 to ca59639 Compare April 11, 2024 17:33
@laduke laduke requested a review from rcoder April 11, 2024 17:36
Copy link

@rcoder rcoder left a comment

Choose a reason for hiding this comment

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

This generally looks pretty good. I'm not totally confident in the current compareVersions implementation and made a suggestion there, but nothing else is a blocker.

test/integration.test.ts Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
test/integration.test.ts Show resolved Hide resolved
Before, we assume that the host/dev env
had a zerotier-one service running.

Now we can start a container and use the
zerotier-one in there, and then throw it away.
Copy link

@rcoder rcoder left a comment

Choose a reason for hiding this comment

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

LGTM

@laduke laduke merged commit ba68ca2 into main Apr 11, 2024
1 check passed
@laduke laduke deleted the laduke/testcontainers branch April 11, 2024 21:14
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.

2 participants