feat(platform-stats): provide number of recently created wikis and users #617
Conversation
m90
force-pushed
the
fr/signup-rate
branch
10 times, most recently
from
14a0366
to
f2eef36
Compare
m90
changed the title
m90
force-pushed
the
fr/signup-rate
branch
9 times, most recently
from
e6403f8
to
1e7a363
Compare
m90
force-pushed
the
fr/signup-rate
branch
2 times, most recently
from
294624f
to
2077e1b
Compare
m90
force-pushed
the
fr/signup-rate
branch
9 times, most recently
from
9c18466
to
fd20ae3
Compare
m90
force-pushed
the
fr/signup-rate
branch
4 times, most recently
from
47bb54b
to
d7c86a2
Compare
m90
force-pushed
the
fr/signup-rate
branch
2 times, most recently
from
af7463e
to
ab41cb5
Compare
| Wiki::query()->delete(); | ||
| User::query()->delete(); | ||
| WikiManager::query()->delete(); | ||
| WikiDb::query()->delete(); |
There was a problem hiding this comment.
I'm not entirely sure why this is still needed when RefreshDatabase is used. If it's not there, each test in this suite will see items created by its preceding siblings.
There was a problem hiding this comment.
This sounds familiar - I think that even may be the reason why we have this target in the Makefile to run tests with a clean database https://github.com/wbstack/api/blob/main/Makefile#L11
(I'm not trying to say this is a good thing - something doesn't seem to work there)
edit: nevermind, I think that was introduced for a different case (running the same test again, not several tests in the same test case)
There was a problem hiding this comment.
I found another trait called DatabaseMigrations: https://laravel.com/docs/8.x/dusk#migrations
It seems we don't need the extended tearDown method when using this trait. (at least the test completes successfully repeatedly with it for me)
I'm not 100% certain this is the right thing to do, as I found this in a browser automation testing section in the laravel docs.
There was a problem hiding this comment.
Not sure what Dusk even is? It might be worth spending some time in removing all statefulness from all tests at some point. It's kind of suboptimal this bites me every single time I add a feature and spend a lot of time debugging why my new test now breaks another test. Ideally, we could even aim for having the tests run using an in-memory database so that we can just run them locally without a database proper.
There was a problem hiding this comment.
There has been work on this here #559 but I'm not sure why it was abandoned.
There was a problem hiding this comment.
In general this looks great to me - as a last step I will try if it works as intended on my local cluster
edit: yay!
[2023-07-03 15:37:17][NwaQTIMNzJXupYp4IsLALN8lYpprPOwe] Processing: App\Jobs\PlatformStatsSummaryJob
{"platform_summary_version":"v1","total":1,"deleted":0,"active":0,"inactive":0,"empty":1,"total_non_deleted_users":0,"total_non_deleted_active_users":0,"total_non_deleted_pages":0,"total_non_deleted_edits":0,"wikis_created_PT24H":1,"users_created_PT24H":1,"wikis_created_P30D":1,"users_created_P30D":1}
[2023-07-03 15:37:18][NwaQTIMNzJXupYp4IsLALN8lYpprPOwe] Processed: App\Jobs\PlatformStatsSummaryJob
| Wiki::query()->delete(); | ||
| User::query()->delete(); | ||
| WikiManager::query()->delete(); | ||
| WikiDb::query()->delete(); |
There was a problem hiding this comment.
I found another trait called DatabaseMigrations: https://laravel.com/docs/8.x/dusk#migrations
It seems we don't need the extended tearDown method when using this trait. (at least the test completes successfully repeatedly with it for me)
I'm not 100% certain this is the right thing to do, as I found this in a browser automation testing section in the laravel docs.
Ticket https://phabricator.wikimedia.org/T335961
This provides any configured number of additional metrics from the
PlatformStatsJob. Ranges are added by passing a validTimeIntervalstring toWBSTACK_SUMMARY_CREATION_RATE_RANGES.