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.

Sign up for GitHub

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

Jump to bottom

e2e test for vagrant setup #224

Merged
merged 2 commits into from
Jul 29, 2020
Merged

e2e test for vagrant setup #224

merged 2 commits into from
Jul 29, 2020

Conversation

gianarb
Copy link
Contributor

@gianarb gianarb commented Jul 20, 2020 •
edited
Loading

Description

This PR uses vagrant to spin up VMs and it execute automatically the Vagrant Setup

Why is this needed

We would like to get automatically tested the Vagrant Setup guide because it has to work in order to offer a great first impression to the community.

Fixes: #169

How Has This Been Tested?

This code tests things by design 😎

How are existing users impacted? What migration steps/scripts do we need?

No impact

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

@gianarb gianarb added size/M estimate of the amount of work to address the issue area/ci-cd-release labels Jul 20, 2020
@mrmrcoleman
Copy link
Contributor

@gianarb It's probably a good idea to update the README to explain a little bit about how this works, in case people need to debug build failures or add new steps.

@gianarb
Copy link
Contributor Author

gianarb commented Jul 20, 2020

Thanks @mrmrcoleman ! The PR does not even work yet! I will be sure to add everything I can add troubleshooting and debug

@mrmrcoleman
Copy link
Contributor

@gianarb molto bene!

@gianarb
Copy link
Contributor Author

gianarb commented Jul 22, 2020

At this point we are in the position where:

go test -v ./test/_vagrant

This command will run the Vagrant Getting Started. The package is called _vagrant because by default go test does not look at packages that start with _. This is an easy trick to exclude tests that should not run as part of the go test ./... made at the root of the project.

@gianarb gianarb added ci-check/vagrant-setup This label trigger a GitHub action that tests the Vagrant Setup guide https://tinkerbell.org/setup/ area/setup Issue related to tinkerbell setup and removed ci-check/vagrant-setup This label trigger a GitHub action that tests the Vagrant Setup guide https://tinkerbell.org/setup/ area/setup Issue related to tinkerbell setup labels Jul 23, 2020
@gianarb gianarb added ci-check/vagrant-setup This label trigger a GitHub action that tests the Vagrant Setup guide https://tinkerbell.org/setup/ and removed ci-check/vagrant-setup This label trigger a GitHub action that tests the Vagrant Setup guide https://tinkerbell.org/setup/ labels Jul 23, 2020
@gianarb
Copy link
Contributor Author

gianarb commented Jul 27, 2020 •
edited
Loading

I realized this PR does not have a lot of sense. The e2e tests use ./setup.sh and it downloads :latest from Docker Registry. So we are not testing against that code.

I still think this is valuable as a workflow and at least as getting started for something that will be useful as soon as we define a release workflow and a starter startup.sh.

In the end the goal was to have a test case for Vagrant and this is what it is.

@gianarb gianarb mentioned this pull request Jul 27, 2020
Closed
thebsdbox
thebsdbox previously approved these changes Jul 27, 2020
View reviewed changes
Copy link
Contributor

@thebsdbox thebsdbox left a comment

Choose a reason for hiding this comment

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

As discussed, I think this is a good phase one as we work towards getting e2e testing enabled for Tinkerbell. This approach will evolve over time as we consider things such as changing the hypervisor to support parallel tests etc.

@gianarb gianarb added the ready-to-merge Signal to Mergify to merge the PR. label Jul 27, 2020
add github workflow
f3e9475 
The GitHub job gets triggered only when the label
`ci-check/vagrant-setup` gets added to a PR.

The job runs on a self-hosted GitHub runner on Packet
@gianarb gianarb added ci-check/vagrant-setup This label trigger a GitHub action that tests the Vagrant Setup guide https://tinkerbell.org/setup/ and removed ci-check/vagrant-setup This label trigger a GitHub action that tests the Vagrant Setup guide https://tinkerbell.org/setup/ labels Jul 29, 2020
@gianarb gianarb requested review from thebsdbox July 29, 2020 13:50
thebsdbox
thebsdbox approved these changes Jul 29, 2020
View reviewed changes
Copy link
Contributor

@thebsdbox thebsdbox left a comment

Choose a reason for hiding this comment

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

/lgtm

@thebsdbox thebsdbox merged commit 64d8cc2 into tinkerbell:master Jul 29, 2020
@gianarb gianarb changed the title [WIP] e2e test for vagrant setup e2e test for vagrant setup Jul 31, 2020
@displague displague mentioned this pull request Aug 3, 2020
Closed
@gianarb gianarb mentioned this pull request Sep 16, 2020
Closed
@mmlb mmlb removed the ready-to-merge Signal to Mergify to merge the PR. label Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thebsdbox thebsdbox thebsdbox approved these changes

@gauravgahlot gauravgahlot Awaiting requested review from gauravgahlot

Assignees
No one assigned
Labels
area/ci-cd-release ci-check/vagrant-setup This label trigger a GitHub action that tests the Vagrant Setup guide https://tinkerbell.org/setup/ size/M estimate of the amount of work to address the issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Smoke or E2E tests
4 participants
@gianarb @mrmrcoleman @thebsdbox @mmlb