-
Notifications
You must be signed in to change notification settings - Fork 10
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
NGSTACK-492 smoke tests #181
Conversation
composer.json
Outdated
"phpunit/phpunit": "^9.5", | ||
"symfony/browser-kit": "5.4.*", | ||
"symfony/css-selector": "5.4.*", | ||
"symfony/phpunit-bridge": "^7.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use 5.4 of the bridge to be inline with other symfony packages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 4381f4f
@@ -66,7 +66,11 @@ | |||
"phpstan/phpstan-strict-rules": "^1.5", | |||
"phpstan/phpstan-symfony": "^1.3", | |||
"phpstan/phpstan-doctrine": "^1.3", | |||
"php-cs-fixer/shim": "^3.50" | |||
"php-cs-fixer/shim": "^3.50", | |||
"phpunit/phpunit": "^9.5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not PHPUnit 10? It supports PHP 8.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, the symfony does not support v10 yet fully: symfony/symfony#49069
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
webpack_encore: | ||
builds: | ||
# The path where Encore is building the assets - i.e. Encore.setOutputPath() | ||
app: '%kernel.project_dir%/public/assets/app/build_dev' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use prod assets in test env to make it a bit faster?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could - but that would requires us to have prod assets ready - which is not necessary at the time of testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not? As you said, the point of these is to run them on deploy. Just make sure tests are ran after building the prod assets, no?
If you run them manually, then yes, you have to build the assets yourself, but I don't see it as an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do - but it is not straightforward.
The idea was to test before doing anything else during deployment so that if it fails, it fails as early as possible. This would mean moving asset build to the beginning of the deploy process, and having to split it into two parts - as right now asset build and copy to prod are bundled together.
It does not seem to me this brings significant performance boost as it is now, but if you'd like, I can make those changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need, it was just an idea.
Maybe later, when and if we have a significant number of tests and if it proves to be a bottleneck.
symfony.lock
Outdated
@@ -1,4 +1,61 @@ | |||
{ | |||
"babdev/pagerfanta-bundle": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be removed from the diff. Only a subset of symfony.lock
is added to this repo in order not to disable fetching latest recipes for each of the new projects.
The subset which is commited is related to keeping the structure of the project inline with ibexa, without Flex interfering with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in 4c47cd
tests/SmokeTests/CrawlerTest.php
Outdated
|
||
foreach($selectedLinkIndices as $index) { | ||
$link = $frontpageLinks[$index]; | ||
fwrite(STDERR, print_r($link->getUri() . PHP_EOL, TRUE)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use Symfony console here? Or see if PHPUnit has it's own output writer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not seem that PHPUnit has anything, but changed to symfony output in 25ea341
{ | ||
$client = parent::createClient($options, $server); | ||
|
||
$server['HTTP_HOST'] = static::getContainer()->getParameter('app.testing.site_domain'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does this parameter come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finished the implementation in c892c41
Deployer now runs multiple times, for all configured domains:
➤ Executing task app:test:phpunit
[localhost] > TEST_DOMAIN=media-site.dev.php81.ez /usr/bin/env php8.1 vendor/bin/phpunit
[localhost] < PHPUnit 9.6.18 by Sebastian Bergmann and contributors.
[localhost] < Testing
[localhost] < Test page: http://media-site.dev.php81.ez/bold_eng/careers?nekisufiks
[localhost] < Test page: http://media-site.dev.php81.ez/fh_eng/recipes/greek-quinoa-salad
[localhost] < Test page: http://media-site.dev.php81.ez/fh_eng/fitness/effects-of-cadence-training-on-running-biomechanics-and-efficiency
[localhost] < .
[localhost] < .
[localhost] < . 3 / 3 (100%)
[localhost] < Time: 00:02.830, Memory: 237.23 MB
[localhost] < OK (3 tests, 9 assertions)
[localhost] < Remaining self deprecation notices (1)
[localhost] < Remaining indirect deprecation notices (437)
[localhost] < Other deprecation notices (66)
[localhost] > TEST_DOMAIN=media-site.dev.php81.ez/bold_eng /usr/bin/env php8.1 vendor/bin/phpunit
[localhost] < PHPUnit 9.6.18 by Sebastian Bergmann and contributors.
[localhost] < Testing
[localhost] < Test page: http://media-site.dev.php81.ez/bold_eng/#
[localhost] < Test page: http://media-site.dev.php81.ez/bold_eng/services
[localhost] < Test page: http://media-site.dev.php81.ez/bold_eng/services
[localhost] < .
[localhost] < .
[localhost] < . 3 / 3 (100%)
[localhost] < Time: 00:01.986, Memory: 209.23 MB
[localhost] < OK (3 tests, 9 assertions)
[localhost] < Remaining self deprecation notices (1)
[localhost] < Remaining indirect deprecation notices (418)
[localhost] < Other deprecation notices (66)
• done on [prod]
✔ Ok [5s 33ms]
All comments have been addressed I believe. |
Thanks @iherak ! |
Introducing three fairly simple "smoke" tests, this is not much, but infinitely better than what we currently have :)
Further implementation, and even proper behat tests will be needed, but this is a first step at least so we have something covered before deploying.