Add GetRoles call to fetch all Grafana roles #151
Conversation
skatsaounis
force-pushed
the
add-get-roles
branch
from
9aabed4
to
c2c47eb
Compare
role_test.go
Outdated
| t.Log(pretty.PrettyFormat(roles)) | ||
|
|
||
| if len(roles) != dashCount { | ||
| t.Errorf("Length of returned roles should be %d", dashCount) |
There was a problem hiding this comment.
I believe this should instead be a call to t.Fatalf, as otherwise if this one fail then it's possible either of the if calls below result in a panic because execution will continue.
There was a problem hiding this comment.
Good point. I just updated the line with your suggestion
| mockData := strings.Repeat(getRoleResponse+",", 1000) // make 1000 roles. | ||
| mockData = "[" + mockData[:len(mockData)-1] + "]" // remove trailing comma; make a json list. | ||
|
|
||
| // This creates 1000 + 1000 + 1 (2001, 3 calls) worth of roles. |
There was a problem hiding this comment.
Can we also test what happens if 0 or any number below 1,000 is returned?
There was a problem hiding this comment.
Both of them are tested with separate test cases with my new commit. Just a comment though. The same test is made for folders and dashboards and these cases are not covered. More or less I copied and modified the logic of those tests.
Hopefully we are good to go now 😄
There was a problem hiding this comment.
Yup, that's better! I'm aware some other tests do not cover all cases (yaaay) but given that we are adding new tests I think it's better if we test them thoroughly.
skatsaounis
force-pushed
the
add-get-roles
branch
from
c2c47eb
to
ed3624f
Compare
skatsaounis
force-pushed
the
add-get-roles
branch
from
d33ae24
to
2081248
Compare
| mockData := strings.Repeat(getRoleResponse+",", 1000) // make 1000 roles. | ||
| mockData = "[" + mockData[:len(mockData)-1] + "]" // remove trailing comma; make a json list. | ||
|
|
||
| // This creates 1000 + 1000 + 1 (2001, 3 calls) worth of roles. |
There was a problem hiding this comment.
Yup, that's better! I'm aware some other tests do not cover all cases (yaaay) but given that we are adding new tests I think it's better if we test them thoroughly.
|
💥 done: v0.22.0 |
Adds method to fetch roles. This will be used by the new
data_source_roleresource of Terraform provider which will fetch roles by name