BCDA-9089: Add IOTA file pattern to cclf parser#1319
Conversation
| assert.NoError(t, err) | ||
| sspProdFile, sspTestFile, sspRunoutFile := gen(sspProd, validTime), gen(sspTest, validTime), | ||
| strings.Replace(gen(sspProd, validTime), "ZC8Y", "ZC8R", 1) | ||
| iotaProdFile, iotaTestFile, iotaRunoutFile := gen(iotaProd, validTime), gen(iotaTest, validTime), strings.Replace(gen(iotaProd, validTime), "ZC8Y", "ZC8R", 1) |
There was a problem hiding this comment.
Maybe Im misreading but IOTAs middle section should be ZC(Y|R), as opposed to some of the others which tracks the CCLF file number (eg ZC8Y). If so, then I would expect this to fail?
There was a problem hiding this comment.
also, it looks like the naming convention doesn't support a performance year. It looks like it's missing?
P.BCD.IOTA***.ZC(Y|R).Dyymmdd.Thhmmsst
There was a problem hiding this comment.
Good catch! I cant load jira right now but it looks like the example was P.BCD.IOTA123.ZCY26.D260206.T1125000. I might have missed that in the regex I put in the ticket.
There was a problem hiding this comment.
Updated the PR and the ticket to match new requirements -- both zip archive name and contained files should track with other cclf file conventions, with an extra "PRT" section in the latter
zip archive: P.BCD.IOTA123.ZCY26.D260206.T1125000
contained file: P.IOTA123.PRT.ZC0Y26.D260201.T0737324
bcda/cclf/parser.go
Outdated
| ssp = `A\d{4}` | ||
| // CCLF filename convention for NGACO: P.V***.ACO.ZC[0|8][Y|R].Dyymmdd.Thhmmsst | ||
| // CCLF file name convention for IOTA with BCD identifier: P.BCD.IOTA***.ZC(Y|R).Dyymmdd.Thhmmsst | ||
| iotaPattern = `IOTA\d{3}` |
There was a problem hiding this comment.
nit: could we rename the other names to follow this convention or could we just call this variable iota?
There was a problem hiding this comment.
Adding to this, isn't the suffix the same as others, too? ZC[0|8](Y|R)(\d{2})
Edit: I am referring to the comment.
There was a problem hiding this comment.
"iota" is actually a keyword in golang 😓
I updated it to "iotaPrt" since it contains an additional "PRT" section
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestGetCMSID(t *testing.T) { |
There was a problem hiding this comment.
Should we add a test case here? for example:
path/T.BCD.IOTA965.ZCY18.D181120.T1000000 → IOTA965
There was a problem hiding this comment.
and maybe a negative test case
P.BCD.IOTA12.ZC8Y24 only 2 digits confirming that IOTA\d{3} is enforced. Totally optional.
There was a problem hiding this comment.
Added a test case here to validate that it parses the cms id correctly.
The service that is responsible for validating that the id is correct is actually the /bcda/service.go --> I added tests there too for positive and negative cases.
|
Im wondering if we should consider tests for things could fail or be mistakenly matched? Eg:
|
michaeljvaldes
requested review from
bhagatparwinder and
laurenkrugen-navapbc
4 participants
🎫 Ticket
https://jira.cms.gov/browse/BCDA-6101
🛠 Changes
ℹ️ Context
🧪 Validation
Unit tests for a straightforward change