[Ref] [Test] Move custom search tests to extension #20990
Conversation
|
(Standard links)
|
|
not any more as per the test failure https://test.civicrm.org/job/CiviCRM-Core-PR/42687/testReport/junit/Civi.Searches/SampleTest/testContactIDs_with_data_set__1/ |
|
erm great - except the test passes locally :-( |
This is a bit of a big lift because I had to re-write the tests a bit to run in the extension context - but all part of the process of core extensionising. Still not gonna tackle another one of the classes today
|
It was a natural order by issue I think |
| @@ -0,0 +1,18 @@ | |||
| <?xml version="1.0"?> | |||
| <phpunit backupGlobals="false" backupStaticAttributes="false" colors="true" convertErrorsToExceptions="true" convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" stopOnFailure="false" bootstrap="tests/phpunit/bootstrap.php"> | |||
There was a problem hiding this comment.
When these tests run do they use this phpunit file? I'll try running locally but as-written I don't think the bootstrap file in this extension would work fully, so I wonder if it's using this or the main core phpunit.xml?
There was a problem hiding this comment.
Yeah I don't think they do but @seamuslee001 might know better
There was a problem hiding this comment.
I think the easiest way to see what I mean is to:
- Drop and recreate your test db, so you're starting fresh from a db that hasn't already run core tests before.
- From the command line,
export CIVICRM_UF=UnitTests phpunit -c /path/to/ext/legacycustomsearches/phpunit.xml.dist /path/to/ext/legacycustomsearches/tests/phpunit/Civi/Searches/SampleTest.php- Crash
Compare the bootstrap.php file in this PR to the main core one in tests/phpunit/Civi/Test. The one generated by civix is a minimal bootstrap that needs editing to run tests fully for a real extension.
There was a problem hiding this comment.
@demeritcowboy no the phpunit.xml.dist that is in the extensions folder is used by the test runner.
The bootstrapping is indeed differently because none of the core tests/ folder is ever bootstrapped for extension phpunit tests only Civi/
The bootstrap.php file matches https://github.com/veda-consulting-company/uk.co.vedaconsulting.mosaico/blob/2.x/tests/phpunit/bootstrap.php and phpunit.xml.dist matches https://github.com/veda-consulting-company/uk.co.vedaconsulting.mosaico/blob/2.x/phpunit.xml.dist with the only exception of not having cacheResult="false"
There was a problem hiding this comment.
Ok so in the environment here I guess it's ok since you know that the db is going to be preloaded with a snapshot. It's just if you ever wanted to run these extension tests standalone/locally, starting from a blank test db, then the bootstrap.php file is incomplete, and/or you need to do a "pre-bootstrap" load of a db snapshot (or which might already be loaded from a core test that you ran earlier). Given how few people are going to be running these tests that seems ok.
There was a problem hiding this comment.
Try those steps I gave above and you'll see what I mean.
There was a problem hiding this comment.
@demeritcowboy no crash for me and these are the steps I did
seamus@civicrmdevelopment:~/buildkit/build/47-test/web/sites/all/modules/civicrm$ civibuild restore --no-civi --no-cms
[[Detected SITE_NAME: 47-test]]
[[Load saved options from /home/seamus/buildkit/build/47-test.sh]]
[[Setup MySQL for Test]]
[[Restore "/home/seamus/buildkit/build/47-test/web" DB (test) from file (/home/seamus/buildkit/app/snapshot/47-test/civi.sql.gz)]]
seamus@civicrmdevelopment:~/buildkit/build/47-test/web/sites/all/modules/civicrm$ export CIVICRM_UF=UnitTests
seamus@civicrmdevelopment:~/buildkit/build/47-test/web/sites/all/modules/civicrm$ phpunit -c /home/seamus/buildkit/build/47-test/web/sites/all/modules/civicrm/ext/legacycustomsearches/phpunit.xml.dist /home/seamus/buildkit/build/47-test/web/sites/all/modules/civicrm/ext/legacycustomsearches/tests/phpunit/Civi/Searches/SampleTest.php
Command 'phpunit' not found, but can be installed with:
sudo apt install phpunit
seamus@civicrmdevelopment:~/buildkit/build/47-test/web/sites/all/modules/civicrm$ phpunit8 -c /home/seamus/buildkit/build/47-test/web/sites/all/modules/civicrm/ext/legacycustomsearches/phpunit.xml.dist /home/seamus/buildkit/build/47-test/web/sites/all/modules/civicrm/ext/legacycustomsearches/tests/phpunit/Civi/Searches/SampleTest.php
Could not read "/home/seamus/buildkit/build/47-test/web/sites/all/modules/civicrm/ext/legacycustomsearches/phpunit.xml.dist".
seamus@civicrmdevelopment:~/buildkit/build/47-test/web/sites/all/modules/civicrm$ git checkout cs
Switched to branch 'cs'
Your branch is up to date with 'eileen/cs'.
seamus@civicrmdevelopment:~/buildkit/build/47-test/web/sites/all/modules/civicrm$ phpunit8 -c /home/seamus/buildkit/build/47-test/web/sites/all/modules/civicrm/ext/legacycustomsearches/phpunit.xml.dist /home/seamus/buildkit/build/47-test/web/sites/all/modules/civicrm/ext/legacycustomsearches/tests/phpunit/Civi/Searches/SampleTest.php
PHPUnit 8.5.15 by Sebastian Bergmann and contributors.
Parsing schema description /home/seamus/buildkit/build/47-test/web/sites/all/modules/civicrm/xml/schema/Schema.xml
Extracting database information
Extracting table information
Installing 47testtest_6zv9y schema
................ 16 / 16 (100%)
Time: 37.25 seconds, Memory: 66.50 MB
OK (16 tests, 35 assertions)
seamus@civicrmdevelopment:~/buildkit/build/47-test/web/sites/all/modules/civicrm$
There was a problem hiding this comment.
Sorry I'm not seeing where you're starting from an empty test db - I do see [[Restore "/home/seamus/buildkit/build/47-test/web" DB (test) from file (/home/seamus/buildkit/app/snapshot/47-test/civi.sql.gz)]] which is preloading a snapshot.
There was a problem hiding this comment.
Should we just merge this as-is for now, given that they're only really going to run in this environment?
There was a problem hiding this comment.
@demeritcowboy I don't think the issues you are raising are specific to this PR so I would say yes
Overview
[Ref] [Test] Move custom search tests to extension
This is a bit of a big lift because I had to re-write the tests a bit to run in the extension context -
but all part of the process of core extensionising. Still not gonna tackle another one of the classes today
Before
Test cover for legacycustomsearches all in core
After
Process of moving started
Technical Details
@seamuslee001 this starts the process of moving the tests over to the extension - are there steps involved in getting CI to run it?
Comments