Various fixes and minor improvements #146
Conversation
| if (!$readable) { | ||
| chmod($filename, 0000); | ||
| if (PHP_OS_FAMILY === 'Windows') { | ||
| exec('icacls ' . escapeshellarg($filename) . ' /deny Everyone:(F)'); |
There was a problem hiding this comment.
Compatibility for Windows (which otherwise kinda ignores chmod()).
| $output = $this->process->getErrorOutput() . $this->process->getOutput(); | ||
|
|
||
| return trim((string) preg_replace('/ +$/m', '', $output)); | ||
| return trim((string) preg_replace(['/ +$/m', '/\r\n/'], ['', "\n"], $output)); |
There was a problem hiding this comment.
Seems that Behat output on windows contains \r\n sequences, which would fail some tests. This change fixes that.
| self::rmdir($file); | ||
| } else { | ||
| unlink($file); | ||
| @unlink($file); |
There was a problem hiding this comment.
A lot of tests failed on this and the changed line below - apparently for some reason it's not able to delete files in temp. Suppressing the error makes the tests pass, leaving behind the files. Normally that's not a big deal since Windows eventually cleans that up.
Using a VFS would solve this and the chmod issue, but I suspect it wouldn't work with exec('behat vfs://...'), unless we add some convoluted extension that transmits the VFS state to behat. Or alternatively, create a temp dir in the current project folder instead of using the system temp dir.
Either way, doesn't feel like a big issue at the moment.
| public function __construct(string $message, int $code = 0, ?Throwable $previous = null, mixed $needle = null, mixed $haystack = null) | ||
| { | ||
| $message .= PHP_EOL . PHP_EOL . sprintf( | ||
| $message .= "\n\n" . sprintf( |
There was a problem hiding this comment.
Fix for Windows, where PHP_EOL returns \r\n causing a string mismatch.
| @@ -0,0 +1,14 @@ | |||
| root = true | |||
There was a problem hiding this comment.
This is to ensure editors/IDEs respect formatting specifics of this project, especially "unusual" things like 2-space indents.
| "sa": "vendor/bin/phpstan", | ||
| "cs": "vendor/bin/php-cs-fixer fix --dry-run --diff", | ||
| "dev": "php -S localhost:8080 -t ./features/bootstrap > build/httpd.log 2>&1", | ||
| "dev": "php -S localhost:8080 -t ./features/bootstrap", |
There was a problem hiding this comment.
I removed that for two reasons:
- compatibility on Windows
- I don't find it really useful - typically these scripts aren't run asynchronously / in-the-background, so there isn't a lot of reason to reroute the output to a file, IMO at least.
| "phpstan/extension-installer": true | ||
| } | ||
| }, | ||
| "process-timeout": 0 |
There was a problem hiding this comment.
Relates to the scripts above - without it they will eventually fail with a timeout (I think after 5 mins or so).
Alternatively, we may want to update the scripts with disableProcessTimeout which makes it more granular and perhaps safer - though I don't see any major disadvantages either way.
| <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.2/phpunit.xsd" executionOrder="depends,defects" | ||
| beStrictAboutOutputDuringTests="true" requireCoverageMetadata="true" | ||
| displayDetailsOnTestsThatTriggerDeprecations="true" displayDetailsOnPhpunitDeprecations="true"> |
There was a problem hiding this comment.
Split to multiple lines for readability
| .vscode | ||
| .php-cs-fixer.cache | ||
| .php-cs-fixer.php | ||
| phpstan.neon |
There was a problem hiding this comment.
This change and renaming the original files is to allow contributors/maintainers to have their own local version of those config files. E.g. a typical case is set up phpstan output formatting to display an IDE link based on your preferred IDE.
| .editorconfig export-ignore | ||
| .gitattributes export-ignore | ||
| .gitignore export-ignore | ||
| .php-cs-fixer.dist.php export-ignore |
There was a problem hiding this comment.
This file is to avoid exporting those entries when creating the composer package, resulting in a faster download/install.
I checked the indentation is consistent and correct... so that must be some sort of GitHub display defect? Ah, I guess it's because of ligatures:
|
@christeredvartsen I just rebased this, so I think it's ready now. The only functional change is in |
1 participant
This PR changes a few (somewhat unrelated) minor things.
Refer to the comments for an explanation.
Everything should be backward compatible.