Skip to content

Run Integration Tests in GitHub #121

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

Merged
merged 21 commits into from
Mar 30, 2021
Merged

Run Integration Tests in GitHub #121

merged 21 commits into from
Mar 30, 2021

Conversation

nhinsch
Copy link
Contributor

@nhinsch nhinsch commented Mar 29, 2021

What does this PR do?

  1. Runs the integration tests as a GitHub action on every commit. Sensitive credentials (AWS secret key/ID, Datadog API key) are passed as secrets. This means that the integration tests will not be able to run for community/non-Datadog-employee pull requests, because we cannot share our these credentials.
  2. Use the stage parameter to create a new/separate CloudFormation stack for each run of the integration tests. This will prevent collisions when the tests are running simultaneously against multiple commits/branches. Delete the CloudFormation stack at the end of the test run. It is possible that the stack will not be deleted if there is an error running the tests or the job is cancelled. These stacks would eventually have to be cleaned up manually, or we could automate the cleanup if this becomes burdensome.
  3. Do not test the Serverless plugin in the Python Library integration tests. Previously, we deployed and tested every combination of event and runtime both by attaching the layer with the Serverless plugin and by attaching the layer directly in serverless.yml. This doubled the overall execution length of the tests, which is already quite long. Also, we should not be testing the Serverless plugin in this manner for a few additional reasons:
    • We should test the plugin when we change the plugin, not when we change the (unrelated) Python Library.
    • The previous tests were extreme overkill for verifying that the plugin worked.
  4. Retry fetching logs if there is a failure. There is an AWS account-wide rate limit on fetching CloudWatch logs; if we run into it, we should retry, not fail the whole test suite.
  5. Move the integration tests to sa-east-1 (previously us-east-1)
  6. Increase the timeout for the integration test Lambda functions to 15 seconds (previously 6 seconds). Previously, the functions did timeout sometimes, and in fact we accidentally committed the timeout errors in the snapshots.

Motivation

Right now, because the integration tests are not run on each commit, it is easy to break the tests (either by introducing an actual bug or simply failing to update the tests) and not know about it for a long time. In fact, the tests are broken as of right now. By running the tests on every commit, we will ensure that they pass.

Types of Changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog
  • This PR passes the integration tests (ask a Datadog member to run the tests)

Copy link
Contributor

@DarcyRaynerDD DarcyRaynerDD left a comment

Choose a reason for hiding this comment

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

LGTM, but i think we should double check the output of the error test function

@nhinsch nhinsch marked this pull request as ready for review March 29, 2021 23:47
@nhinsch nhinsch requested a review from a team as a code owner March 29, 2021 23:47
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