Skip to content

CircleCI integration tests w/o Chef#27

Merged
hmac merged 1 commit intomasterfrom
lawrence-circleci-workflow-v2
Jan 25, 2018
Merged

CircleCI integration tests w/o Chef#27
hmac merged 1 commit intomasterfrom
lawrence-circleci-workflow-v2

Conversation

@lawrencejones
Copy link
Copy Markdown
Contributor

@lawrencejones lawrencejones commented Dec 12, 2017

Remove the dependence on the draupnir Chef cookbook to run integration
tests, vastly simplifying and increasing the speed of running tests from
a local development machine.

This is achieved by using plain dockerfiles and a wrapper in the rspec
test suite to provision a new draupnir instance each time the tests are
run. We also introduce a CircleCI workflow that splits compilation of
the go binary, unit testing of said binary and running rspec integration
tests into separate dependent jobs.

@lawrencejones lawrencejones force-pushed the lawrence-circleci-workflow-v2 branch 2 times, most recently from 5db4362 to 50cee7c Compare December 18, 2017 09:53
@lawrencejones
Copy link
Copy Markdown
Contributor Author

@hmac I'd love a cursory review on this whenever you have the time!

Comment thread .circleci/Dockerfile
ruby-dev \
&& sudo gem install bundler fpm \
&& sudo apt-get clean -y \
&& sudo rm -rf /var/lib/apt/lists/*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Specifically the rm? because you want your container to be as small as possible, and this removes all the junk that apt downloads when you interact with the repositories (that you don't need to run your container)

Comment thread spec/fixtures/bootstrap Outdated
cp /workspace/spec/fixtures/key.pem /etc/ssl/certs/draupnir_key.pem

# Install scripts, boot draupnir
find /workspace/cmd -type f -exec cp -v {} /usr/bin/ \;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could this not just be cp /workspace/cmd/* /usr/bin/?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I wanted to be specific about what was being copied in, only bringing files across. We can change this though, it probably doesn't get us much.

Copy link
Copy Markdown
Contributor

@hmac hmac Jan 8, 2018

Choose a reason for hiding this comment

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

what else could you bring across that wasn't files? I don't think cp will copy directories without -R

Comment thread spec/spec_helper.rb
end
end
end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not a huge fan of having to refer to @draupnir everywhere in specs. Can we not just define the three http methods here?

def client
  @client ||= Draupnir.create_from_container.tap { |c| raise "draupnir did not boot" unless c.alive? }
end

def get(path)
  client.request(:get, path)
end

def post(path, payload, headers={})
  client.request(:post, path, payload, headers)
end

def delete(path)
  client.request(:delete, path)
end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

How come you don't like the @draupnir in the specs? Can I argue devils advocate that it makes it very clear from just a glance which parts of the spec are interacting with our test instance?

I'm not attached either way but I slightly favour having one handle to a class, rather than many distinct methods.

Copy link
Copy Markdown
Contributor

@hmac hmac Jan 3, 2018

Choose a reason for hiding this comment

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

It's noisy and unhelpful - the entire test suite involves hitting the service with http calls, so it doesn't distinguish anything. draupnir is also not a helpful name, since it refers to the project as a whole. It'd be like using the variable gocardless in our integration tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This also begs the question about how we handle booting and destroying the instance in tests- whether we want to pay the price to boot an instance each time or whether it's best to boot one and deal with it if we break that instance in the middle of a test suite.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's fair, I think it makes it quite difficult to see what you're actually doing when calling the get/post/delete methods though! Will make the change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

On a side note, I think @draupnir is more honest in how it exposes the underlying mechanism to the test code, in that it makes it clear there is a global handle with state being passed around.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's a piece of state that doesn't change throughout the lifetime of the test suite though. This is exactly equivalent to spinning up some service (e.g. elasticsearch) before running your integration tests.

Comment thread Makefile
bundle exec kitchen exec -c "sudo sh -c \"echo 'DRAUPNIR_ENVIRONMENT=test' >> /etc/environments/draupnir.env\""
bundle exec kitchen exec -c "sudo cp /vagrant/fixtures/cert.pem /etc/ssl/certs/draupnir_cert.pem"
bundle exec kitchen exec -c "sudo cp /vagrant/fixtures/key.pem /etc/ssl/certs/draupnir_key.pem"
bundle exec kitchen exec -c "sudo service draupnir start"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

very happy to see all this go

Comment thread Makefile
update-cookbook:
cd tmp/cookbooks/draupnir && git pull && rm -rf berks-cookbooks && bundle && bundle exec berks vendor


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

and especially this

@lawrencejones lawrencejones force-pushed the lawrence-circleci-workflow-v2 branch from 50cee7c to cb4984a Compare January 20, 2018 14:21
@lawrencejones lawrencejones changed the base branch from lawrence-vendor-deps to master January 20, 2018 14:23
@lawrencejones lawrencejones force-pushed the lawrence-circleci-workflow-v2 branch from cb4984a to 0b838c2 Compare January 20, 2018 14:27
@lawrencejones lawrencejones changed the title [WIP] CircleCI workflow v2 CircleCI integration tests w/o Chef Jan 20, 2018
@lawrencejones lawrencejones force-pushed the lawrence-circleci-workflow-v2 branch from 0b838c2 to d6907a2 Compare January 20, 2018 14:36
@lawrencejones
Copy link
Copy Markdown
Contributor Author

This is good for a final review now @hmac

Comment thread Dockerfile Outdated
software-properties-common \
build-essential \
curl \
wget \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reckon we can do without one of these

@lawrencejones lawrencejones force-pushed the lawrence-circleci-workflow-v2 branch from d6907a2 to 77bd9e1 Compare January 22, 2018 20:29
Remove the dependence on the draupnir Chef cookbook to run integration
tests, vastly simplifying and increasing the speed of running tests from
a local development machine.

This is achieved by using plain dockerfiles and a wrapper in the rspec
test suite to provision a new draupnir instance each time the tests are
run. We also introduce a CircleCI workflow that splits compilation of
the go binary, unit testing of said binary and running rspec integration
tests into separate dependent jobs.
@lawrencejones lawrencejones force-pushed the lawrence-circleci-workflow-v2 branch from 77bd9e1 to 7c0786d Compare January 22, 2018 20:36
@hmac hmac merged commit 9525814 into master Jan 25, 2018
@hmac hmac deleted the lawrence-circleci-workflow-v2 branch January 25, 2018 09:04
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