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

Move integration tests to integration package #2009

Merged
merged 24 commits into from
Dec 13, 2024
Merged

Move integration tests to integration package #2009

merged 24 commits into from
Dec 13, 2024

Conversation

pietern
Copy link
Contributor

@pietern pietern commented Dec 12, 2024

Changes

Objectives:

  • A dedicated directory for integration tests
  • It is not picked up by go test ./...
  • No need for a TestAcc test name prefix
  • More granular packages to improve test selection (future)

The tree structure generally mirrors the source code tree structure.

Requirements for new files in this directory:

  • Every package must be named after its directory with _test appended
    • Requiring a different package name for integration tests avoids aliasing with the main package.
  • Every integration test package must include a main_test.go file.

These requirements are enforced by a unit test in the integration package.

Tests

Integration tests pass.

The total run time regresses by about 10%. A follow-up change that increases the degree of test parallelism will address this.

integration/README.md Show resolved Hide resolved
integration/README.md Show resolved Hide resolved
integration/README.md Outdated Show resolved Hide resolved
.vscode/settings.json Outdated Show resolved Hide resolved
integration/bundle/artifacts_test.go Outdated Show resolved Hide resolved
@pietern pietern requested a review from denik December 13, 2024 12:48
@@ -0,0 +1,20 @@
package internal
Copy link
Contributor

@denik denik Dec 13, 2024

Choose a reason for hiding this comment

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

nit: I find "internal" not very descriptive. "helpers" or "utils" or "setuputil" would be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can move it to a package inside internal. We do need the internal nesting because it means it can only be imported by other packages inside integration. See https://go.dev/doc/go1.4#internalpackages

Copy link

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 2009
  • Commit SHA: da2d3fdda4da9960686fed0cb1ba0c2b16153c06

Checks will be approved automatically on success.

@eng-dev-ecosystem-bot
Copy link
Collaborator

Test Details: go/deco-tests/12317614117

@pietern
Copy link
Contributor Author

pietern commented Dec 13, 2024

One additional test skip in the last commit. Merging without waiting for another integration test run.

@pietern pietern merged commit c958702 into main Dec 13, 2024
10 checks passed
@pietern pietern deleted the move-integration branch December 13, 2024 14:39
// TestMain is the entrypoint executed by the test runner.
// See [internal.Main] for prerequisites for running integration tests.
func TestMain(m *testing.M) {
internal.Main(m)
Copy link
Contributor

Choose a reason for hiding this comment

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

@pietern we might want to re-think this - it turns out having TestMain disables go test's built-in caching, which could be useful in speeding up test runs both locally and on CI.

shreyas-goenka added a commit that referenced this pull request Jan 2, 2025
## Changes
This PR adds back debugging functionality that was lost during migration
to `internal.Main` as an entry point for integration tests.

The PR that caused the regression:
#2009. Specifically the addition
of internal.Main as the entrypoint for all integration tests.

## Tests
Manually, by trying to debug a test.
github-merge-queue bot pushed a commit that referenced this pull request Jan 8, 2025
## Changes
- Remove TestMain from integration tests and related checks.
- This fixes "go test" caching for integration tests.

The test_main.go files were added in
#2009 to make sure integration
tests are not run as part of go test ./.... We recommend running make
test to run tests, which includes the packages to test (and excludes
integration).

## Tests
To test that caching works I ran a test twice:

```
+ CLOUD_ENV=aws
+ go test --timeout 3h -v -run TestDefaultPython/3.9 ./integration/bundle/
…
PASS
ok      github.com/databricks/cli/integration/bundle    (cached)
```
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.

3 participants