- Notifications
You must be signed in to change notification settings - Fork674
feat(api): add application statistics#2347
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
feat(api): add application statistics#2347
Uh oh!
There was an error while loading.Please reload this page.
Conversation
0ff7047 to090ed9aCompareShreya-7 commentedOct 29, 2022
@nejch had a quick question. I've written a functional test for this functionality where I'm creating some entities (projects, groups, snippets, users, etc.) and then checking whether the statistics returned match or not (number of projects created = number from statistics). I have two options here: either I go around modifying those tests to clean up any entities they create, or I do a count of created (and undeleted) entities and just modify my test accordingly. |
lmilbaum commentedOct 29, 2022
Jumping in if I may. The fixture |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Shreya-7 commentedOct 29, 2022
Just what I was looking for, thanks! |
codecov-commenter commentedOct 29, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## main #2347 +/- ##======================================= Coverage 95.78% 95.79% ======================================= Files 79 79 Lines 5249 5258 +9 =======================================+ Hits 5028 5037 +9 Misses 221 221
Flags with carried forward coverage won't be shown.Click here to find out more.
|
nejch commentedOct 30, 2022
Sorry@Shreya-7 I was away on a long weekend and catching up a bit!
I would actually keep it even simpler than that, and only check that the attributes in the response can be successfully cast to an int but not actually assert on their values, that way we know the server response makes sense. GitLab does a lot of creation and cleanup tasks asynchronously and I would not rely on absolute values here. We've actually tried to move away from that in some other tests - see#2118 for context. That said, I see you've done a lot of work on getting |
Uh oh!
There was an error while loading.Please reload this page.
Shreya-7 commentedOct 31, 2022
This makes sense, let me try that!
So should I open a new PR for this fix? I'm fine with it going in with this one, since my test does use this function |
nejch left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@Shreya-7 up to you if you want this as a separate PR, I'm fine with it in here too! Just a few more nits from me here.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
56f11fa to6fcf3b6Compare
nejch left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for all the work here@Shreya-7! Hopefully next time it's less of a bumpy ride ;)
Uh oh!
There was an error while loading.Please reload this page.
resolves#2264