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.

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

somewhat fix phpunit so it could run functional tests again #396

Merged
merged 1 commit into from
Feb 18, 2024

Conversation

asdfzdfj
Copy link
Contributor

@asdfzdfj asdfzdfj commented Jan 4, 2024

who tests the test infrastructure

not sure what happened but it seems to be a combination of:

  • symfony/phpunit-bridge doesn't support phpunit 10 yet
  • but we're using phpunit 10 anyway
  • applied multiple config recipes and overwritten a decent chunk out of them
  • dama/doctrine-test-bundle not active when running tests, leading to db error when testing due to data conflict and mismatch between tests data in db and what the tests sees and has access to

included fixes:

  • applied a small phpunit config changes just enough to get it back up and running again
  • included auto test database bootstrap for phpunit, activate by setting BOOTSTRAP_DB env variable when starting tests (e.g. BOOTSTRAP_DB=1 ./bin/phpunit)

@asdfzdfj asdfzdfj added the please review I want to be sure what I did won't cause problems label Jan 4, 2024
@melroy89
Copy link
Member

melroy89 commented Jan 9, 2024

What does make sense is to add the functional tests to github workflow.

@e-five256
Copy link
Member

e-five256 commented Jan 9, 2024

I think we'd need sqlite support or something, I could be mistaken but I recall similar conversations about setting up dependencies and sqlite not supporting the specific postgres features we use. I think that can be out of scope for this, I'd just like to verify the functional tests work correctly with this change but am having trouble myself getting them to run, which probably isn't a good sign >.> making it possible in docker might be nice so system setup isn't a requirement, but again not putting that on asdfzdfj, just brainstorming ways to both learn myself how to run the tests and instruct others

@melroy89
Copy link
Member

melroy89 commented Jan 9, 2024

I think we'd need sqlite support or something, I could be mistaken but I recall similar conversations about setting up dependencies and sqlite not supporting the specific postgres features we use. I think that can be out of scope for this, I'd just like to verify the functional tests work correctly with this change but am having trouble myself getting them to run, which probably isn't a good sign >.> making it possible in docker might be nice so system setup isn't a requirement, but again not putting that on asdfzdfj, just brainstorming ways to both learn myself how to run the tests and instruct others

GitHub should allow us to use a PostgreSQL docker container as a dependency for a specific GitHub Action Job: https://docs.github.com/en/actions/using-containerized-services/creating-postgresql-service-containers. Allowing us to just leverage PostgreSQL and run the tests in the same matter as you might do locally on your dev system.

@asdfzdfj
Copy link
Contributor Author

asdfzdfj commented Jan 10, 2024

What does make sense is to add the functional tests to github workflow.

perhaps, I'm not familiar with how github workflow/ci works and it's a bit out of scope for this patch so feel free to make a patch doing that if you want to add this

still, I'd recommended to not run the full tests all the time, as it's a really expensive operation, using quite a bit of memory and time (I had to raise my php mem limit to 1G for phpunit to not fail halfway due to running out memory, also it took about 22m to complete on my dinky R5 5600G machine), at the very least I'd like this to be run before cutting a new release so regression could be fixed before actually making a release

@asdfzdfj asdfzdfj force-pushed the fix/phpunit10-run-functional-test branch from be0ecda to b802ed2 Compare February 16, 2024 16:42
@melroy89 melroy89 added this to the v1.4.0 milestone Feb 18, 2024
who tests the test infrastructure

not sure what happened but it seems to be a combination of:
- symfony/phpunit-bridge doesn't support phpunit 10 yet
- but we're using phpunit 10 anyway
- applied multiple config recipes and overwritten a decent chunk out of
  them
- dama/doctrine-test-bundle not active when running tests, leading to
  db error when testing due to data conflict and mismatch between tests
  data in db and what the tests sees and has access to

included fixes:
- applied a small phpunit config changes just enough to get it back up
  and running again
- included auto test database bootstrap for phpunit, activate by setting
  BOOTSTRAP_DB env variable when starting tests
  (e.g. `BOOTSTRAP_DB=1 ./bin/phpunit`)
@asdfzdfj asdfzdfj force-pushed the fix/phpunit10-run-functional-test branch from b802ed2 to 604fe7d Compare February 18, 2024 15:35
@asdfzdfj asdfzdfj merged commit ab35790 into main Feb 18, 2024
7 checks passed
@asdfzdfj asdfzdfj deleted the fix/phpunit10-run-functional-test branch February 18, 2024 15:37
@melroy89
Copy link
Member

@asdfzdfj Follow up could be to briefly document the BOOTSTRAP_DB=1 ./bin/phpunit usage, in CONTRIBUTING.md?

andrewmoise pushed a commit to andrewmoise/mbin that referenced this pull request Feb 20, 2024
)

who tests the test infrastructure

not sure what happened but it seems to be a combination of:
- symfony/phpunit-bridge doesn't support phpunit 10 yet
- but we're using phpunit 10 anyway
- applied multiple config recipes and overwritten a decent chunk out of
  them
- dama/doctrine-test-bundle not active when running tests, leading to
  db error when testing due to data conflict and mismatch between tests
  data in db and what the tests sees and has access to

included fixes:
- applied a small phpunit config changes just enough to get it back up
  and running again
- included auto test database bootstrap for phpunit, activate by setting
  BOOTSTRAP_DB env variable when starting tests
  (e.g. `BOOTSTRAP_DB=1 ./bin/phpunit`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please review I want to be sure what I did won't cause problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants