Skip to content
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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tests for stats listener #99

Merged
merged 34 commits into from
Feb 23, 2024
Merged

Tests for stats listener #99

merged 34 commits into from
Feb 23, 2024

Conversation

psrpinto
Copy link
Member

@psrpinto psrpinto commented Feb 19, 2024 •
edited
Loading

This PR adds tests for the Stats_Listener (which "listens" to actions from GlotPress).

This took me longer than expected because it turns out that the test setup wasn't yet fully done. I looked at how https://github.com/GlotPress/gp-translation-helpers does it but since that repo doesn't use wp-env, it didn't directly apply to here, so I had to figure some things out.

TODO

  • Make tests that need GlotPress run on wp-env
  • Make tests that need GlotPress run on CI
  • Write tests for Stats_Listener

@psrpinto psrpinto changed the base branch from trunk to namespaces February 19, 2024 13:02
@psrpinto psrpinto linked an issue Feb 19, 2024 that may be closed by this pull request
Add unit tests for stats #15
Closed
Base automatically changed from namespaces to trunk February 19, 2024 16:48
@psrpinto psrpinto force-pushed the tests-stats branch 6 times, most recently from 52b1da9 to 4842b19 Compare February 21, 2024 16:02
@psrpinto psrpinto self-assigned this Feb 21, 2024
@psrpinto psrpinto force-pushed the tests-stats branch 7 times, most recently from 3b854c5 to e490c42 Compare February 21, 2024 18:15
@psrpinto psrpinto force-pushed the tests-stats branch 2 times, most recently from e08c4f8 to 4abc92f Compare February 22, 2024 11:30
@psrpinto psrpinto changed the title Tests for stats Tests for stats listener Feb 22, 2024
@psrpinto psrpinto marked this pull request as ready for review February 22, 2024 15:42
akirk
akirk requested changes Feb 23, 2024
View reviewed changes
Copy link
Member

@akirk akirk left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests work well and pass, great job there! But I am not a fan of the local wp-env dependency. If I add what I suggested in my comment it can run without wp-env and by just running ./vendor/bin/phpunit

Can we find a way that allows for both?

tests/bootstrap.php Outdated Show resolved Hide resolved
@psrpinto
No need to install GlotPress
f86cab8 
We're calling glotpress/tests/phpunit/includes/loader.php which already installs glotpress.
@psrpinto
Apply schema in bootstrap.php
16b424d
@psrpinto
No longer need the run-tests.sh script
4d6a81e
@psrpinto psrpinto requested a review from akirk February 23, 2024 14:33
akirk
akirk approved these changes Feb 23, 2024
View reviewed changes
Copy link
Member

@akirk akirk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the improvement!

@psrpinto psrpinto merged commit b266815 into trunk Feb 23, 2024
2 checks passed
@psrpinto psrpinto deleted the tests-stats branch February 23, 2024 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akirk akirk akirk approved these changes

@trymebytes trymebytes Awaiting requested review from trymebytes

@amieiro amieiro Awaiting requested review from amieiro

Assignees

@psrpinto psrpinto

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add unit tests for stats
2 participants
@psrpinto @akirk