Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
#2469713-158: Step 2: Create a JavaScriptTestBase using PhantomJs Driver/Binary requires us to run just the javascript tests.
Proposed resolution
Possible solutions:
- Allow to filter by base class: Doesn't work because this requires reflection, which blows up on our codesize
- Expose the testsuite as flag. While this would work, it would add a new flag
- Map Phpunit test suites to simpletest groups and leverage the
--group
functionality. Changes how tests are grouped - Add a new
type
key to the simpletest information collected for each test class
Remaining tasks
Review, RTBC, Commit
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#29 | 2659100-29.patch | 8.88 KB | alexpott |
#29 | 25-29-interdiff.txt | 5.24 KB | alexpott |
#25 | 2659100-25.patch | 6.21 KB | dawehner |
#23 | interdiff.txt | 714 bytes | dawehner |
#23 | 2659100-22.patch | 17.88 KB | dawehner |
Comments
Comment #2
dawehnerComment #3
jibranDo we need some kind of test here?
Comment #4
dawehnerWe could test this part, but this seems to be about it.
Comment #6
jibranThis test case looks fine to me.
Comment #7
dawehnerYeah, well, let's use the existing one.
Comment #8
jibranLooks good just have to uncomment some code.
Comment #9
dawehnerComment #10
jibranThank you. This is ready now. Do we need a change notice for this?
Comment #11
dawehnerI think we should write a change record for actual javascript tests, but not for this particular small change.
Comment #12
alexpottAre we sure that this is the best way to go? Can't we use a special namespace like we're done with PHPUnit tests to get them to be run? And aren't we going to need to exclude these tests from a usual run?
I think we need to broaden this issue to do a bit more. Once we have javascript testing we have three test environments... simpletest, phpunit, and javascript (or maybe these are runners). Further more each environment can run different test types and these different types have different requriements:
I think as a start we need to be able to define which test environments we're running so the following works as expected...
Comment #13
jibranWe are not adding javascript tests here. We are adding javascript functional tests support afaik. Why would we use php to write JS test? We should use mocha, jUnit or etc. for that.
We'll run functional tests on Phantomjs which is merely another mink driver just like goutte or selenium. So to put it correctly
Maybe IS doesn't explain completely what we are doing here. Unless I'm missing something.
Comment #14
Mile23I haven't been following the JS testing stuff, and I don't know much about JS testing, so I have a few naive questions:
Are we supporting core tests here, or adding options so that others can do stuff?
It looks like phantomjs has to be installed somewhere to run these, so how do we check for the dependency in the runner?
Do these tests fail or skip if the dependencies aren't met?
Do we have a reporting layer to talk back to the testbot and eventually d.o?
If there were a change notice or some other documentation to link to, that would help.
Comment #15
dawehnerMaybe a crazy suggestion, what about reading on them first?
All what you are talk about don't belong into this issue but rather #2469713: Step 2: Create a JavaScriptTestBase using PhantomJs Driver/Binary. I tried to make progress by opening a new issue and not implement this for the other issue as well.
IMHO Its tricky. I mean we could split up tests in php and javascript. We would split it up in unit, kernel and functional. At least on the longrun when we support javascript unit testing, IMHO we should make it possible to run just the javascript unit tests, and maybe the PHP functional tests. The split you suggested just seems weird for me.
Comment #16
Mile23I did, which is why I asked those questions.
Last I looked the mink-based testing setup was basically experimental, which is why I'm wondering about whether this is for core or not. So I'm basically echoing @alexpott: "javascript has ?"
The answer seems to be that it's the same mink-flavored testing stuff, just with a JS head, so yay. Now: What happens if that infrastructure isn't present?
And here's a review:
Call it @javascript-functional or something, since we'll eventually have JS unit tests.
Same deal. We'll have pure JS tests later and we might run them from run-tests.sh. Rename to --js-functional or something?
We should also have a --functional flag. I bet there's an issue for it.
Comment #17
larowlanJust playing devil's advocate here...
Should we instead be looking at a real php-based build tool, like phing or robo.
Taking the phing approach (cause thats what I'm most familiar with, no other reason) - this would mean:
The bot would stick a build.properties in the checkout containing the environment variables such as db connection and build-artifacts directory as needed.
Then it would run one command -
phing
- which would spit out artifacts into the nominated directory, which jenkins would then pick up.But this one command would be made up of a number of phing calls (e.g. it would run phpcs, unit tests, functional tests, javascript tests, integration tests).
If you're running locally and want to run javascript tests or unit tests, or code-coverage for example:
phing javascript
phing unit
phing phpcs
phing code-coverage
And so on.
That way we wouldn't rely on run-tests.sh to run phpunit tests - we would instead run them direct with the phpunit runner and the junit xml results printer (which jenkins understands).
In addition, we don't need to remember all the flags required -
phing -l
gives you your options - want to see what commands a build is running?phing unit -verbose
The exit code of the phpunit script would bubble to the phing/robo runner and determine the build status and we'd get away from all the code in simpletest module that wraps phpunit.
I think this is the ultimate end-point of the new CI, and in that ultimate end-game, run-tests.sh should only run legacy simpletest tests - at least in my opinion.
There was a session on this at Prague 2013 by @boztek - https://prague2013.drupal.org/session/have-build-and-automate-it-phing.html
Comment #18
dawehnerWell, my idea was to also run the javascript only unit tests in the future with that flag.
Comment #19
dawehner@larowlan
I would agree with in general. Having one tool simplifies people to use when our testsuite grows and grows. The problem though is, changing anything on the testbot is hard
and seems much harder than what we could do in core, so for now we should support things in run-tests.sh and then maybe slowly move over.
Sadly there is a bit of a problem with running phpunit natively, because when it fatals it just dies completely. I guess when we use phing we could also provide a wrapper around it, so we can always at least write some xml file, so we can communicate the pure failure back to the testbot
Comment #20
dawehnerAs a first step, let's expose the testsuites we have in core in the groups. This would allow us then to run what we need.
Testbot would then run:
Comment #21
Mile23Yes please. :-)
Reminds me of this: #2605664: [Needs change record updates] Align TestDiscovery and bootstrap.php's non-permissive loading of PSR-4 namespaces for traits
Comment #22
alexpottI think this approach is okay. Whilst I'm not wild about changing groups of everything it is the best compromise for progress.
This comment needs updating
Comment #23
dawehnerThank you @alexpott
Comment #24
jibranI think patch includes #2469713: Step 2: Create a JavaScriptTestBase using PhantomJs Driver/Binary now.
Comment #25
dawehnerWell yeah, it contains that, because I wanted to try it out as part of it
Comment #26
dawehnerComment #27
jibranSorry forgot to RTBC earlier.
Comment #28
alexpottSo this patch makes the groups displayed in Simpletest UI less useful - we have the problem that we still only support one group per test. We should have addressed this when we added the PHPUnit group but we never did. I think we should address #2296615: Accurately support multiple @groups per test class first BUT it is super annoying to hold back javascript testing. None of the compromises here feel good :(
Comment #29
alexpottSo thinking about this some more I think we shouldn't have overridden the group concept for phpunit. It was a mistake to override this so people could run all the PHPunit tests simply through the UI. If you want to run all the PHPunit tests just use the phpunit runner from CLI.
So therefore I think we should just add a
type
key to the test information. That way we can repurpose #2670978: Allow to run just specific types when running all tests to add the ability to specify test types to run when using run-tests.sh.@dawehner I'm sorry for the change of direction again and I tried to reach out on IRC before posting this.
Comment #30
dawehnerI totally agree with that route of solving the issue. Less compromises, same flexibility.
Comment #31
alexpottComment #32
jibranCan we quickly add PHPUnit-FunctionalJavascript test as well?
Comment #33
alexpott@jibran imo that is covered already.
Comment #34
jibranRight but once testing suite and other is testing provided info. Anyway if you don't feel the need then it's fine by me.
This is getting test suite not phpunittest suite.
Comment #37
catchCommitted/pushed to 8.1.x and cherry-picked to 8.0.x, thanks!
Comment #39
Mile23Some of the tests added here are broken: #2893371: Several methods theoretically added to TestInfoParsingTest were actually not