fix: Tests for plugins

[harsh-2711]

What does this do / why do we need it?

The PR fixes the getTotalNodes permission test which was failing due to the version change of es from 6.x to 7.x

What should your reviewer look out for in this PR?

The path in server setup struct on which the query is tested

Do you need help or clarification on anything?

No

Which issue(s) does this PR fix?

#16

[jeet-parekh]

All of your changes work.
But there are a few tests that don't work for me in those files - unrelated to your changes. Could you check those?

TestGetRawLogs in plugins/logs/dao_test.go

TestGetRawOwnerPermissions in plugins/permissions/dao_test.go

TestGetUser and TestDeleteUser in plugins/users/dao_test.go

[harsh-2711]

@jeet-parekh I am working on it. Still, the issue isn't completely solved. I will complete it within 4 days

[harsh-2711]

@jeet-parekh I have fixed all the tests except 1 case in TestGetRawLogs in plugins/logs/dao_test.go and TestGetRawOwnerPermissions in plugins/permissions/dao_test.go. Only 1 from 4 test cases are failing for these two tests with the same error json: cannot unmarshal number into Go struct field SearchHits.hits.total of type elastic.TotalHits. Can you help me with this?

[jeet-parekh]

@harsh-2711, I'll take a closer look soon.
plugins/users/dao_test.go -> TestDeleteUser still fails for me in cases when getUserTest shows nil.

deleteUser should have failed with error: missing required fields: [Type] got: <nil> instead

[jeet-parekh]

@harsh-2711, In permissions/dao_test.go,
TestPatchPermission, TestPostPermission, and TestGetRawOwnerPermissions have incorrect statements in their logs. Could you fix that first?

[harsh-2711]

@harsh-2711, In permissions/dao_test.go,
TestPatchPermission, TestPostPermission, and TestGetRawOwnerPermissions have incorrect statements in their logs. Could you fix that first?

@jeet-parekh Done. I missed that 😔

[harsh-2711]

@harsh-2711, I'll take a closer look soon.
plugins/users/dao_test.go -> TestDeleteUser still fails for me in cases when getUserTest shows nil.

deleteUser should have failed with error: missing required fields: [Type] got: <nil> instead

Yes. It's confusing why ES is not returning any type missing error when Search is made without any type.

[jeet-parekh]

@harsh-2711, @siddharthlatest,

https://www.elastic.co/guide/en/elasticsearch/reference/current/breaking-changes-7.0.html#hits-total-now-object-search-response

It says that hits.total is supposed to be an object now. But when I run these tests against elasticsearch:7.3.1 docker, I get hits.total as a number. Could you check what you are getting?

[jeet-parekh]

I think I figured it out. ES7 works the way it should. But I think we are using mocks for testing. Right?

Inside each ServerSetup.Response of getRawLogsTest, change the format of hits.total to the format mentioned in the link I shared above. And it should work then.

[harsh-2711]

@jeet-parekh Thanks for help. I'll do it.

[harsh-2711]

@jeet-parekh I am not getting any response after making a request using function es.getRawLogs(context.Background(), tt.from, tt.size, "index1") which consists of

response, err := es.client.Search(es.indexName).
		Query(query).
		From(offset).
		Size(s).
		Sort("timestamp", false).
		Do(ctx)

The function is returning error after this point

[jeet-parekh]

@harsh-2711, is it the same error as before?

[harsh-2711]

@harsh-2711, is it the same error as before?

Yes

[jeet-parekh]

Inside each ServerSetup.Response of getRawLogsTest, change the format of hits.total to the format mentioned in the link I shared above. And it should work then.

strange, those errors disappeared for me after this ☝️

[harsh-2711]

Can you add the changes in this branch? I'll test them once. Because they aren't working for me

[jeet-parekh]

sure

[jeet-parekh]

@harsh-2711, I pushed a change which solves

json: cannot unmarshal number into Go struct field SearchHits.hits.total of type elastic.TotalHits

[harsh-2711]

Done👍🏻 Thanks @jeet-parekh

[jeet-parekh]

@harsh-2711, no worries... so only TestDeleteUser remains pending now

[jeet-parekh]

@harsh-2711, @siddharthlatest, could this be the reason?
https://www.elastic.co/guide/en/elasticsearch/reference/current/removal-of-types.html

[siddharthlatest]

@jeet-parekh Not sure of the context. Reason for what?

This line does look problematic for ES v7: https://github.com/appbaseio/arc/pull/25/files#diff-44bc5029f965a2bfbec785db82de0f39R201.

[jeet-parekh]

@siddharthlatest,
TestDeleteUser in plugins/users/dao_test.go fails with the following error.

deleteUser should have failed with error: missing required fields: [Type] got: <nil> instead

I was looking for a reason for that. And that link is the closest thing I found. You know better about ES. What do you think?

[jeet-parekh]

@harsh-2711, try Siddharth's suggestion

[harsh-2711]

@jeet-parekh All tests are passing now. Please take a look.

[jeet-parekh]

The tests work fine. Could you check and correct the name parameter that you pass inside t.Run inside each test? Many of them are wrong.

Other than that, it LGTM.

[harsh-2711]

@jeet-parekh Test headers are changed now

[siddharthlatest]

closing this, as we have moved to using end-to-end tests instead of mock tests.