Skip to content

BCDA-9392: Test Container Database #1207

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

Merged
merged 21 commits into from
Sep 22, 2025
Merged

BCDA-9392: Test Container Database #1207

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
4f9382e
poc database container for testing
laurenkrugen-navapbc Sep 9, 2025
f92ae69
pr feedback
laurenkrugen-navapbc Sep 9, 2025
7775b92
spelling
laurenkrugen-navapbc Sep 9, 2025
36ad040
test fixes, todo fix, general cleanup
laurenkrugen-navapbc Sep 11, 2025
80fe33c
remove explicit docker host
laurenkrugen-navapbc Sep 11, 2025
2e949f7
add extra hosts
laurenkrugen-navapbc Sep 11, 2025
a1d39e8
add container tests and small fixes
laurenkrugen-navapbc Sep 12, 2025
b6ccc80
pr feedback
laurenkrugen-navapbc Sep 15, 2025
f7d29cc
Merge branch 'main' into lauren/BCDA-9392
bhagatparwinder Sep 15, 2025
9159592
snyk updates
laurenkrugen-navapbc Sep 15, 2025
6b94ed8
snyk updates
laurenkrugen-navapbc Sep 15, 2025
2e14939
snyk updates
laurenkrugen-navapbc Sep 15, 2025
9472174
dependency updates
laurenkrugen-navapbc Sep 15, 2025
1558104
dep update
laurenkrugen-navapbc Sep 15, 2025
90503e8
updating go pkgs to mitigate snyk findngs
bhagatparwinder Sep 16, 2025
96c3e0f
updating go pkgs to mitigate snyk findngs
bhagatparwinder Sep 16, 2025
13c325c
Revert "updating go pkgs to mitigate snyk findngs"
bhagatparwinder Sep 16, 2025
0f600c8
Revert "updating go pkgs to mitigate snyk findngs"
bhagatparwinder Sep 16, 2025
3c6da88
update postgres version to const
laurenkrugen-navapbc Sep 17, 2025
a0bb100
Merge branch 'main' into lauren/BCDA-9392
bhagatparwinder Sep 17, 2025
3ad83b9
Merge branch 'main' into lauren/BCDA-9392
laurenkrugen-navapbc Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 56 additions & 16 deletions bcda/api/v2/api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,24 @@ import (
"github.com/CMSgov/bcda-app/bcda/auth"
"github.com/CMSgov/bcda-app/bcda/client"
"github.com/CMSgov/bcda-app/bcda/constants"
"github.com/CMSgov/bcda-app/bcda/database"
"github.com/CMSgov/bcda-app/bcda/models"
"github.com/CMSgov/bcda-app/bcda/models/postgres"
"github.com/CMSgov/bcda-app/bcda/models/postgres/postgrestest"
"github.com/CMSgov/bcda-app/bcda/service"
"github.com/CMSgov/bcda-app/bcda/testUtils"
"github.com/CMSgov/bcda-app/bcda/utils"
"github.com/CMSgov/bcda-app/bcda/web/middleware"
"github.com/CMSgov/bcda-app/conf"
"github.com/CMSgov/bcda-app/db"
"github.com/CMSgov/bcda-app/log"
appMiddleware "github.com/CMSgov/bcda-app/middleware"
_ "github.com/golang-migrate/migrate/v4/database/postgres"
_ "github.com/golang-migrate/migrate/v4/source/file"
"github.com/sirupsen/logrus"

"github.com/go-chi/chi/v5"
_ "github.com/golang-migrate/migrate/v4/database/postgres"
_ "github.com/golang-migrate/migrate/v4/source/file"
"github.com/google/fhir/go/fhirversion"
"github.com/google/fhir/go/jsonformat"
fhircodes "github.com/google/fhir/go/proto/google/fhir/proto/r4/core/codes_go_proto"
Expand All @@ -40,6 +45,7 @@ import (
"github.com/pborman/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
)

Expand All @@ -53,15 +59,16 @@ var (

type APITestSuite struct {
suite.Suite
db *sql.DB
pool *pgxv5Pool.Pool
apiV2 *ApiV2
db *sql.DB
pool *pgxv5Pool.Pool
apiV2 *ApiV2
dbContainer db.TestDatabaseContainer
}

func (s *APITestSuite) SetupSuite() {
s.db = database.Connect()
s.pool = database.ConnectPool()
s.apiV2 = NewApiV2(s.db, s.pool)
ctr, err := db.NewTestDatabaseContainer()
require.NoError(s.T(), err)
s.dbContainer = ctr

origDate := conf.GetEnv("CCLF_REF_DATE")
conf.SetEnv(s.T(), "CCLF_REF_DATE", time.Now().Format("060102 15:01:01"))
Expand All @@ -81,8 +88,36 @@ func (s *APITestSuite) SetupSuite() {
client.SetLogger(log.BFDAPI)
}

func (s *APITestSuite) SetupTest() {
var err error
s.db, err = s.dbContainer.NewSqlDbConnection()
require.NoError(s.T(), err)
s.pool, err = s.dbContainer.NewPgxPoolConnection()
require.NoError(s.T(), err)
s.apiV2 = NewApiV2(s.db, s.pool)

}

func (s *APITestSuite) SetupSubTest() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Learned something interesting about our table driven tests.

When iterating over the list of sub tests, if the Run command's func is invoked with testing.T, instead of left empty, it will not recognize the tests in the table as subtests.

// subtests not recognized
s.Run(string(tt.status), func(t *testing.T) {
...
}

// subtests recognized
s.Run(string(tt.status), func() {
...
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you go over sub tests? Is that when something is run through via a helper suite or when iterating through a range of tests? Is the goal of leaving it empty so that the suite recognizes it as a subtest and therefore runs the SetupSubTest code? If it doesnt recognize it as a subtest would it just run the SetupTest code (and therefore we wouldnt need both setup functions)?

It seems like we are doing the same thing in SetupTest and SetupSubTest. Thinking we will likely want to do these things across all test suites. Maybe we can break this off into a helper function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Subtests are just passed in functions to suite.Run().
  2. Yup, the goal is if it's left empty, instead of *testing.T, it picks runs it as a subtest.
  3. Correct - it will run the SetupTest and TeardownTest, which means the subtests will not have clean states.
  4. Because the setup and teardown for tests and subtests will vary across the test suites and are required on a per suite basis, it seems like doing the test setup/teardown in those functions would make the most sense?

This also might be a good time to have a convo about separating out unit vs integration and talking about overall best practices. Because this suite has tests with subtests and some without, we have to do the setup in both vs separating out the test suite into components. We don't really have a set standard, but something for future discussion?

var err error
s.db, err = s.dbContainer.NewSqlDbConnection()
require.NoError(s.T(), err)
s.pool, err = s.dbContainer.NewPgxPoolConnection()
require.NoError(s.T(), err)
s.apiV2 = NewApiV2(s.db, s.pool)

}

func (s *APITestSuite) TearDownTest() {
postgrestest.DeleteJobsByACOID(s.T(), s.db, acoUnderTest)
s.db.Close()
err := s.dbContainer.RestoreSnapshot("Base")
require.NoError(s.T(), err)
}

func (s *APITestSuite) TearDownSubTest() {
s.db.Close()
err := s.dbContainer.RestoreSnapshot("Base")
require.NoError(s.T(), err)
}

func TestAPITestSuite(t *testing.T) {
Expand Down Expand Up @@ -140,29 +175,34 @@ func (s *APITestSuite) TestJobStatusNotComplete() {
}

for _, tt := range tests {
s.T().Run(string(tt.status), func(t *testing.T) {
s.Run(string(tt.status), func() {
j := models.Job{
ACOID: uuid.Parse("DBBD1CE1-AE24-435C-807D-ED45953077D3"),
RequestURL: constants.V2Path + constants.PatientEOBPath,
Status: tt.status,
CreatedAt: time.Now(),
UpdatedAt: time.Now(),
}
postgrestest.CreateJobs(t, s.db, &j)
defer postgrestest.DeleteJobByID(t, s.db, j.ID)
var err error
r := postgres.NewRepository(s.db)
j.ID, err = r.CreateJob(context.Background(), j)
require.NoError(s.T(), err)

req := s.createJobStatusRequest(acoUnderTest, j.ID)
rr := httptest.NewRecorder()

s.apiV2.JobStatus(rr, req)
assert.Equal(t, tt.expStatusCode, rr.Code)
assert.Equal(s.T(), tt.expStatusCode, rr.Code)
switch rr.Code {
case http.StatusAccepted:
assert.Contains(t, rr.Header().Get("X-Progress"), tt.status)
assert.Equal(t, "", rr.Header().Get("Expires"))
assert.Contains(s.T(), rr.Header().Get("X-Progress"), tt.status)
assert.Equal(s.T(), "", rr.Header().Get("Expires"))
case http.StatusInternalServerError:
assert.Contains(t, rr.Body.String(), "Service encountered numerous errors")
assert.Contains(s.T(), rr.Body.String(), "Service encountered numerous errors")
case http.StatusGone:
assertExpiryEquals(t, j.CreatedAt.Add(s.apiV2.handler.JobTimeout), rr.Header().Get("Expires"))
assertExpiryEquals(s.T(), j.CreatedAt.Add(s.apiV2.handler.JobTimeout), rr.Header().Get("Expires"))
}

})
}
}
Expand Down
47 changes: 47 additions & 0 deletions db/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Test Database Container
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking this should be in a different dir. When I think of the db dir I think of db migrations, sql scripts, etc, not a go package aimed at testing. Maybe something like test_container?

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 put it in here because it does utilize the db migrations and sql scripts, but the primary reason was that db does not depend on any other local packages, so it would be a safe import.

I am up for putting it in another package, as long as the package does not import any other local packages.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a new package? While I agree, this is testing, but it is specifically for the database. Would we ever see this being used outside of DB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If this were in it's own package, it would still need access to all the files under db/. Because we have some high level packages and this would be used in both the api and the worker, it made sense to have it here, I think.


## Purpose
To have an idempotent database for each test run.

## Implementation Strategy

`TestDatabaseContainer` is a lightweight wrapper of https://golang.testcontainers.org/modules/postgres/. Because BCDA uses pgx, pgxpool, and sql/db for database connections, this wrapper type will implement methods to utilize any of the aforementioned connection types.

This type also implements methods to apply migrations and seed the database with an initial set of necessary data for the BCDA application to execute database operations.


## How To Use

1. Create the container in the setup of the test suite; this is the longest running step.
2. Create the database connection in the setup of the test or the setup of the subtest.
3. (optional) Seed any additional test data with TestDatabaseContainer.ExecuteFile() or TestDatabaseContainer.ExecuteDir(). For details on seed data, please consult the README.md in ./seeddata
4. Restore a snapshot in the test teardown.

*Note*: Database snapshots cannot be created or restored if a database connection still exists.

```
type FooTestSuite struct {
suite.Suite
db *sql.DB // example; pgx or pool can also be used
dbContainer db.TestDatabaseContainer. // example; this is optional to be part of the test suite
}
func (s *FooTestSuite) SetupSuite() {
ctr, err := db.NewTestDatabaseContainer()
require.NoError(s.T(), err)
s.dbContainer = ctr
}
func (s *FooTestSuite) SetupTest() {
db, err := s.dbContainer.NewSqlDbConnection()
require.NoError(s.T(), err)
s.db = db
}
func (s *FooTestSuite) TearDownTest() {
s.db.Close()
err := s.dbContainer.RestoreSnapshot(). // example, you can restore from another desired snapshot
require.NoError(s.T(), err)
}
```
Loading
Loading