feat: SSAPI Unit Tests (BPS-272) #1982
Conversation
Caleb-Hurshman
requested review from
schmikei
and removed request for
a team and
dpaasman00
Caleb-Hurshman
changed the title
| require.False(t, done) | ||
| } | ||
|
|
||
| func TestInitCheckpoint(t *testing.T) { |
There was a problem hiding this comment.
Are these supposed to be implemented?
There was a problem hiding this comment.
Somehow those changes didn't get added in my last commit. Should be good now.
| Type string `xml:"type,attr"` | ||
| Dict Dict `xml:"dict"` | ||
| } `xml:"content"` | ||
| type SearchStatusResponse struct { |
There was a problem hiding this comment.
| type SearchStatusResponse struct { | |
| type SearchJobStatusResponse struct { |
nit: I think this is more indicative of a name. not required just something I think makes it more readable
| } | ||
|
|
||
| // Content struct to represent <content> elements | ||
| type Content struct { |
There was a problem hiding this comment.
| type Content struct { | |
| type SearchJobContent struct { |
Along similar veins
| </s:key> | ||
| </s:dict> | ||
| </content> | ||
| </entry> |
There was a problem hiding this comment.
| </entry> | |
| </entry> | |
nit
| @@ -11,6 +11,7 @@ require ( | |||
| go.opentelemetry.io/collector/extension/experimental/storage v0.113.0 | |||
| go.opentelemetry.io/collector/pdata v1.19.0 | |||
| go.opentelemetry.io/collector/receiver v0.113.0 | |||
| go.opentelemetry.io/collector/receiver/receivertest v0.113.0 | |||
There was a problem hiding this comment.
Did this not rmake any go.sum changes? I'd imagine it would. Would you mind re-running go mod tidy to just double check?
| ) | ||
|
|
||
| var ( | ||
| server = newMockServer() |
There was a problem hiding this comment.
nit I think I'd like to see these defined within the test functions themselves so other test files can use the names server and testClient without worrying about variable shadowing.
| <s:key name="cursorTime">1969-12-31T16:00:00.000-08:00</s:key> | ||
| <s:key name="delegate"></s:key> | ||
| <s:key name="diskUsage">2174976</s:key> | ||
| <s:key name="dispatchState">DONE</s:key> |
There was a problem hiding this comment.
I think having a dispatchState as not done will be another important test for any future changes.
| @@ -60,6 +61,10 @@ func newSplunkSearchAPIClient(ctx context.Context, settings component.TelemetryS | |||
| func (c defaultSplunkSearchAPIClient) CreateSearchJob(search string) (CreateJobResponse, error) { | |||
| endpoint := fmt.Sprintf("%s/services/search/jobs", c.endpoint) | |||
|
|
|||
| if !strings.Contains(search, "starttime=") || !strings.Contains(search, "endtime=") || !strings.Contains(search, "timeformat=") { | |||
There was a problem hiding this comment.
Might be overkill but in splunk, are these case-insensitive? I.e. I believe you can do Starttime="<whatever> and its still valid
For the sake of correctness I'm curious if we need to make these error checks non case-sensitive?
I know that we're inputting these parameters so its not too big of a deal, so maybe a comment would suffice in this situation
There was a problem hiding this comment.
Good catch, I didn't know that about Splunk.
Proposed Change
Add more unit tests for the Splunk Search API Receiver.
Checklist