Skip to content
This repository has been archived by the owner on Aug 12, 2022. It is now read-only.

feat(tests): Fetch only the resources required for test being run #400

Merged
merged 3 commits into from
Jul 4, 2022

Conversation

hermanschaaf
Copy link
Member

@hermanschaaf hermanschaaf commented Jul 1, 2022

Summary

This closes #398

When adding a new resource, it is desirable to only fetch that resource during integration testing. Before this change, all resources were fetched (which took about 3.5 minutes). After the change, only the resources required by the subtest are fetched (so test runs for cq-provider-aws now take about 10 seconds on my machine, when limiting to a specific table).

For example, when running integration tests with:

go test -v -run="TestIntegration/aws_lightsail_instances" -tags=integration ./...

only the resources required to populate the aws_lightsail_instances and related tables will be fetched. If a subtest is not being run, all resources will still be fetched. This means integration tests for a specific resource can be run in about 10 seconds, as opposed to the several minutes it took before. The trade-off is, however, that the full suite will take slightly longer to run. I think this is worthwhile trade-off to make, as long as we're not also jeopardizing correctness. I think this could happen if there are dependencies between resources, but hopefully the tests for this PR will fail now if this is the case.


Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines 🧑‍🎓
  • Run go fmt to format your code 🖊
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests 🧪
  • Ensure the status checks below are successful ✅

@hermanschaaf hermanschaaf requested a review from a team as a code owner July 1, 2022 15:23
@hermanschaaf hermanschaaf requested review from erezrokah and removed request for a team July 1, 2022 15:23
@yevgenypats
Copy link
Member

Looks good! Maybe we should pass the resource name instead the table name to t.Run - this should make things simpler and will avoid any reverse-lookup for table <-> resource because there can be multiple nested tables in one resource

Copy link
Member

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

Amazing 🔥 To fix the lint errors you'll need to run gci write . (https://github.com/daixiang0/gci)

@hermanschaaf
Copy link
Member Author

Looks good! Maybe we should pass the resource name instead the table name to t.Run

@yevgenypats I've made this change now. We'll have to update some examples in some plugin repos to use the new format, I'll open new PRs for those.

@hermanschaaf hermanschaaf merged commit 5fa0315 into cloudquery:main Jul 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(integration tests): Integration tests should only fetch the relevant resources
3 participants