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.

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

Jump to bottom

BCDA 7122: add test coverage and cclf files #867

Merged
merged 4 commits into from
Sep 22, 2023
Merged

BCDA 7122: add test coverage and cclf files #867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2c067d1
add test coverage and cclf files
alex-dzeda Sep 19, 2023
154e484
add consistent style to TestImportCCLFDirectory
alex-dzeda Sep 19, 2023
9c56f50
separate tests to identify which is failing on GH Actions
alex-dzeda Sep 20, 2023
9748f24
remove empty directory test, failing upstream
alex-dzeda Sep 20, 2023
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
230 changes: 212 additions & 18 deletions bcda/cclf/cclf_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,25 @@ import (
"archive/zip"
"context"
"database/sql"
"io/ioutil"
"os"
"path/filepath"
"sort"
"strings"
"testing"
"time"

"github.com/pborman/uuid"
"github.com/sirupsen/logrus/hooks/test"

"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/log"

"github.com/CMSgov/bcda-app/bcda/testUtils"

"github.com/CMSgov/bcda-app/conf"
"github.com/CMSgov/bcda-app/log"
"github.com/pborman/uuid"
"github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"

"github.com/CMSgov/bcda-app/bcda/database"
"github.com/CMSgov/bcda-app/bcda/models"
"github.com/CMSgov/bcda-app/conf"
)

type CCLFTestSuite struct {
Expand All @@ -51,7 +46,7 @@ func (s *CCLFTestSuite) SetupTest() {
func (s *CCLFTestSuite) SetupSuite() {
s.origDate = conf.GetEnv("CCLF_REF_DATE")

dir, err := ioutil.TempDir("", "*")
dir, err := os.MkdirTemp("", "*")
if err != nil {
s.FailNow(err.Error())
}
Expand Down Expand Up @@ -81,6 +76,10 @@ func (s *CCLFTestSuite) TestImportCCLF0() {
cclf0filePath := filepath.Join(s.basePath, "cclf/archives/valid/T.BCD.A0001.ZCY18.D181120.T1000000")
cclf0metadata := &cclfFileMetadata{env: "test", acoID: "A0001", cclfNum: 0, timestamp: time.Now(), filePath: cclf0filePath, perfYear: 18, name: "T.BCD.A0001.ZC0Y18.D181120.T1000011"}

// Missing metadata
_, err := importCCLF0(ctx, nil)
assert.EqualError(err, "file CCLF0 not found")

// positive
validator, err := importCCLF0(ctx, cclf0metadata)
assert.Nil(err)
Expand All @@ -102,6 +101,25 @@ func (s *CCLFTestSuite) TestImportCCLF0() {
cclf0metadata = &cclfFileMetadata{env: "test", acoID: "A0001", cclfNum: 0, timestamp: time.Now(), filePath: cclf0filePath, perfYear: 18, name: "T.BCD.A0001.ZC0Y18.D181120.T1000013"}
_, err = importCCLF0(ctx, cclf0metadata)
assert.EqualError(err, "duplicate CCLF8 file type found from CCLF0 file")

// missing file
cclf0filePath = filepath.Join(s.basePath, "cclf/archives/0/missing_data/T.BCD.A0001.ZCY18.D181122.T1000000")
cclf0metadata = &cclfFileMetadata{env: "test", acoID: "A0001", cclfNum: 0, timestamp: time.Now(), filePath: cclf0filePath, perfYear: 18, name: "Z.BCD.A0001.ZC0Y18.D181120.T1000013"}
_, err = importCCLF0(ctx, cclf0metadata)
assert.Nil(err)

//invalid record count
cclf0filePath = filepath.Join(s.basePath, "cclf/archives/0/invalid/T.A0001.ACO.ZC0Y18.D181120.Z1000000")
cclf0metadata = &cclfFileMetadata{env: "test", acoID: "A0001", cclfNum: 0, timestamp: time.Now(), filePath: cclf0filePath, perfYear: 18, name: "T.A0001.ACO.ZC0Y18.D181120.Z1000011"}
_, err = importCCLF0(ctx, cclf0metadata)
assert.EqualError(err, "failed to parse CCLF8 record count from CCLF0 file: strconv.Atoi: parsing \"N\": invalid syntax")

//invalid record length
cclf0filePath = filepath.Join(s.basePath, "cclf/archives/0/invalid/T.BCD.ACOB.ZC0Y18.D181120.E0001000")
cclf0metadata = &cclfFileMetadata{env: "test", acoID: "A0001", cclfNum: 0, timestamp: time.Now(), filePath: cclf0filePath, perfYear: 18, name: "T.A0001.ACO.ZC0Y18.D181120.E1000011"}
_, err = importCCLF0(ctx, cclf0metadata)
assert.EqualError(err, "failed to parse CCLF8 record length from CCLF0 file: strconv.Atoi: parsing \"Num\": invalid syntax")

}

func (s *CCLFTestSuite) TestImportCCLF0_SplitFiles() {
Expand All @@ -115,43 +133,154 @@ func (s *CCLFTestSuite) TestImportCCLF0_SplitFiles() {
assert.Equal(cclfFileValidator{totalRecordCount: 6, maxRecordLength: 549}, validator["CCLF8"])
}

func (s *CCLFTestSuite) TestImportCCLFDirectoryValid() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Golang has a feature called table driven testing, where you can run boiler plate code with different inputs and expected outcomes/outputs. We use this in some areas of our codebase, like in the fileprocessor.

Table driven testing doesn't necessarily offer any improvements to performance and unless we're testing a handful of different inputs/outputs then it's not terribly useful.

There are some places like TestImportCCLFDirectoryValid, TestImportCCLFDirectoryInvalid, TestImportCCLFDirectoryTwoLevels, and this func where it might come in handy, but that's totally up to you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, Lauren! I'm going to check out table driven testing and see if I can apply it to some of the upcoming test coverage tickets.

assert := assert.New(s.T())
//Happy case, with directory containing valid BCD files.
_, _, _, err := ImportCCLFDirectory(filepath.Join(s.basePath, constants.CCLFDIR, "archives", "valid_bcd"))
assert.Nil(err)
}
func (s *CCLFTestSuite) TestImportCCLFDirectoryInvalid() {
assert := assert.New(s.T())
//Directory with mixed file types + at least one bad file.
cclfDirectory := filepath.Join(s.basePath, constants.CCLFDIR)
_, _, _, err := ImportCCLFDirectory(cclfDirectory)
assert.EqualError(err, "one or more files failed to import correctly")

//Target bad file directory
cclfDirectory = filepath.Join(s.basePath, constants.CCLFDIR, "archives", "invalid_bcd")
_, _, _, err = ImportCCLFDirectory(cclfDirectory)
assert.EqualError(err, "one or more files failed to import correctly")

}

/*
func (s *CCLFTestSuite) TestImportCCLFDirectoryEmpty() {
assert := assert.New(s.T())
//Zero CCLF files in directory
cclfDirectory := filepath.Join(s.basePath, constants.CCLFDIR, "emptydir")
_, _, _, err := ImportCCLFDirectory(cclfDirectory)
assert.Nil(err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Did this test not work? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one was funky; it passed locally using both go test, as well as the make unit-test, but github actions was having none of it, even after several attempts to mitigate (eg making file paths os-agnostic)

*/
func (s *CCLFTestSuite) TestImportCCLFDirectoryTwoLevels() {
assert := assert.New(s.T())
//Zero CCLF files in directory
//additional invalid directory
cclfDirectory := filepath.Join(s.basePath, constants.CCLFDIR, "emptydir", "archives")
_, _, _, err := ImportCCLFDirectory(cclfDirectory)
assert.EqualError(err, "error in sorting cclf file: nil,: lstat "+cclfDirectory+": no such file or directory")

}

func (s *CCLFTestSuite) TestValidate() {
ctx := context.Background()
assert := assert.New(s.T())

cclf8filePath := filepath.Join(s.basePath, constants.CCLF8CompPath)
cclf8metadata := &cclfFileMetadata{env: "test", acoID: "A0001", cclfNum: 8, timestamp: time.Now(), filePath: cclf8filePath, perfYear: 18, name: constants.CCLF8Name}

// positive
// missing metadata
cclfvalidator := map[string]cclfFileValidator{"CCLF8": {totalRecordCount: 7, maxRecordLength: 549}}
err := validate(ctx, cclf8metadata, cclfvalidator)
err := validate(ctx, nil, cclfvalidator)
assert.EqualError(err, "file not found")

// positive
cclfvalidator = map[string]cclfFileValidator{"CCLF8": {totalRecordCount: 7, maxRecordLength: 549}}
err = validate(ctx, cclf8metadata, cclfvalidator)
assert.Nil(err)

// negative
cclfvalidator = map[string]cclfFileValidator{"CCLF8": {totalRecordCount: 2, maxRecordLength: 549}}
err = validate(ctx, cclf8metadata, cclfvalidator)
assert.EqualError(err, "maximum record count reached for file CCLF8 (expected: 2, actual: 3)")

//invalid cclfNum
cclf8metadata = &cclfFileMetadata{env: "test", acoID: "A0001", cclfNum: 9, timestamp: time.Now(), filePath: cclf8filePath, perfYear: 18, name: constants.CCLF8Name}
err = validate(ctx, cclf8metadata, cclfvalidator)
assert.EqualError(err, "unknown file type when validating file: T.BCD.A0001.ZC8Y18.D181120.T1000009")

//invalid file path
cclf8metadata = &cclfFileMetadata{env: "test", acoID: "A0001", cclfNum: 8, timestamp: time.Now(), filePath: "/", perfYear: 18, name: constants.CCLF8Name}
err = validate(ctx, cclf8metadata, cclfvalidator)
assert.EqualError(err, "could not read archive /: read /: is a directory")

//non-existant file
cclf8metadata = &cclfFileMetadata{env: "test", acoID: "A0001", cclfNum: 8, timestamp: time.Now(), filePath: cclf8filePath, perfYear: 18, name: "InvalidName"}
err = validate(ctx, cclf8metadata, cclfvalidator)
assert.Nil(err)

//more records than expected
cclfvalidator = map[string]cclfFileValidator{"CCLF8": {totalRecordCount: 7, maxRecordLength: 0}}
cclf8metadata = &cclfFileMetadata{env: "test", acoID: "A0001", cclfNum: 8, timestamp: time.Now(), filePath: cclf8filePath, perfYear: 18, name: constants.CCLF8Name}
err = validate(ctx, cclf8metadata, cclfvalidator)
assert.EqualError(err, "incorrect record length for file CCLF8 (expected: 0, actual: 549)")

}

func (s *CCLFTestSuite) TestImportCCLF8() {
assert := assert.New(s.T())

//indeterminate test results without deletion of both.
postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, "A0001")
defer postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, "A0001")
postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, "A0002")
defer postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, "A0002")

acoID := "A0001"
fileTime, _ := time.Parse(time.RFC3339, constants.TestFileTime)

//invalid directory
metadata := &cclfFileMetadata{
name: constants.CCLF8Name,
env: "test",
acoID: acoID,
cclfNum: 8,
perfYear: 18,
timestamp: fileTime,
filePath: "/missingdir",
}
err := importCCLF8(context.Background(), metadata)
assert.EqualError(err, "could not read CCLF8 archive /missingdir: open /missingdir: no such file or directory")

//No files in CCLF zip
metadata = &cclfFileMetadata{
name: constants.CCLF8Name,
env: "test",
acoID: acoID,
cclfNum: 8,
perfYear: 18,
timestamp: fileTime,
filePath: filepath.Join(s.basePath, "cclf/archives/8/invalid/T.BCD.A0001.ZCY18.D181125.T1000087"),
}
err = importCCLF8(context.Background(), metadata)
assert.EqualError(err, "no files found in CCLF8 archive "+metadata.filePath)

//file not found
metadata = &cclfFileMetadata{
name: constants.CCLF8Name + "E",
env: "test",
acoID: acoID,
cclfNum: 8,
perfYear: 18,
timestamp: fileTime,
filePath: filepath.Join(s.basePath, constants.CCLF8CompPath),
}

err := importCCLF8(context.Background(), metadata)
err = importCCLF8(context.Background(), metadata)
assert.EqualError(err, "file "+metadata.name+" not found in archive "+metadata.filePath)

//successful
metadata = &cclfFileMetadata{
name: constants.CCLF8Name,
env: "test",
acoID: acoID,
cclfNum: 8,
perfYear: 18,
timestamp: fileTime,
filePath: filepath.Join(s.basePath, constants.CCLF8CompPath),
}

err = importCCLF8(context.Background(), metadata)
if err != nil {
s.FailNow("importCCLF8() error: %s", err.Error())
}
Expand All @@ -177,6 +306,40 @@ func (s *CCLFTestSuite) TestImportCCLF8() {
assert.Equal("1A69B98CD35", mbis[5])
}

func (s *CCLFTestSuite) TestImportCCLF8DBErrors() {
assert := assert.New(s.T())

//indeterminate test results without deletion of both.
postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, "A0001")
defer postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, "A0001")
postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, "A9999")
defer postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, "A9999")

defer postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, "A0002")
//postgrestest.DeleteCCLFFilesByCMSID())

acoID := "A0001"
fileTime, _ := time.Parse(" ", " ")

//metadata from success case
metadata := &cclfFileMetadata{
name: constants.CCLF8Name,
env: "test",
acoID: acoID,
cclfNum: 8,
perfYear: 18,
timestamp: fileTime,
filePath: filepath.Join(s.basePath, constants.CCLF8CompPath),
}
//Send an invalid context to fail DB check
ctx, function := context.WithCancel(context.TODO())
function()

err := importCCLF8(ctx, metadata)
assert.EqualError(err, "failed to check existence of CCLF8 file: context canceled")

}

func (s *CCLFTestSuite) TestImportCCLF8_alreadyExists() {
assert := assert.New(s.T())

Expand Down Expand Up @@ -234,7 +397,6 @@ func (s *CCLFTestSuite) TestImportCCLF8_Invalid() {
// This error indicates that we did not supply enough characters for the MBI
assert.Contains(err.Error(), "invalid byte sequence for encoding \"UTF8\": 0x00")
}

func (s *CCLFTestSuite) TestCleanupCCLF() {
assert := assert.New(s.T())
cclfmap := make(map[string]map[metadataKey][]*cclfFileMetadata)
Expand Down Expand Up @@ -280,13 +442,45 @@ func (s *CCLFTestSuite) TestCleanupCCLF() {
filePath: filepath.Join(s.basePath, "cclf/archives/valid/T.BCD.A0001.ZCY18.D181122.T1000000"),
imported: true,
}

cclfmap["A0001"] = map[metadataKey][]*cclfFileMetadata{
{perfYear: 18, fileType: models.FileTypeDefault}: {cclf0metadata, cclf8metadata, cclf9metadata},
}
err := cleanUpCCLF(context.Background(), cclfmap)
assert.Nil(err)

files, err := ioutil.ReadDir(conf.GetEnv("PENDING_DELETION_DIR"))
//negative cases
//File unable to be renamed
fileTime, _ = time.Parse(time.RFC3339, constants.TestFileTime)
cclf10metadata := &cclfFileMetadata{
name: "T.BCD.A0001.ZC9Y18.D181120.T1000010",
env: "test",
acoID: acoID,
cclfNum: 10,
perfYear: 18,
timestamp: fileTime,
filePath: filepath.Join(s.basePath, "cclf/archives/valid/T.BCD.A0001.ZCY18.D181122.Z1000000"),
imported: true,
}
//unsuccessful, not imported
fileTime, _ = time.Parse(time.RFC3339, constants.TestFileTime)
cclf11metadata := &cclfFileMetadata{
name: "T.BCD.A0001.ZC9Y18.D181120.T1000010",
env: "test",
acoID: acoID,
cclfNum: 10,
perfYear: 18,
timestamp: fileTime,
filePath: filepath.Join(s.basePath, "cclf/archives/valid/T.BCD.A0001.ZCY18.D181122.Z1000000"),
imported: false,
}
cclfmap["A0001"] = map[metadataKey][]*cclfFileMetadata{
{perfYear: 18, fileType: models.FileTypeDefault}: {cclf10metadata, cclf11metadata},
}
err = cleanUpCCLF(context.Background(), cclfmap)
assert.EqualError(err, "2 files could not be cleaned up")

files, err := os.ReadDir(conf.GetEnv("PENDING_DELETION_DIR"))
if err != nil {
s.FailNow("failed to read directory: %s", conf.GetEnv("PENDING_DELETION_DIR"), err)
}
Expand Down Expand Up @@ -339,7 +533,7 @@ func (s *CCLFTestSuite) TestImportRunoutCCLF() {
func createTemporaryCCLF8ZipFile(t *testing.T, data string) (fileName, cclfName string) {
cclfName = uuid.New()

f, err := ioutil.TempFile("", "*")
f, err := os.CreateTemp("", "*")
assert.NoError(t, err)

w := zip.NewWriter(f)
Expand Down
Loading
Loading