Closed timothylcooke closed 2 years ago
Many thanks, @timothylcooke!
I just approved running the automated tests for this pull request. Looks like some failures there which will need looking into - are you able to do that? If you're unable to see the errors, please let me know.
Also, could you please add a line to the CHANGELOG file in project and @
yourself for contributing the fix?
No problem. Thanks for the repo. I'd be happy to edit the changelog, and I can definitely fix the linting/code quality issues.
From what I can tell, the integration-tests are failing with the message Error: "[PHP_FPM]" "[ERROR] Another FPM instance seems to already listen on ././php/fpm.sock"
, and I can't imagine that my change would break that.
I can take another swing at this tomorrow and actually do it right, but I'm not sure if I'll be able to fix the integration-tests issue on my own as I have zero experience with PHP. To be frank, I'm not really interested in messing around with getting a working dev environment for hours that I'm only going to use once to fix one line in an open source repo. If I can figure it out in an hour or two, I'll be happy to help out.
@leonstafford I fixed the linting issues and updated . I can't run the test-integration script on Windows, but I suspect it will fail with the same error as last time that FPM is already listening, since my changes to whitespace wouldn't affect that. Linting passes, but composer run-script test
fails with error:
There was 1 failure:
1) WP2Static\FileHelperTest::testGetListOfLocalFilesByDir
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
Array (
- 0 => '/top_level_latin_folder/no_file_extension'
- 1 => '/top_level_latin_folder/example_of_an_extremely_long_latin_file_name_with_some_numbers_at_the_end_0123456789.fileextension'
+ 0 => '/top_level_latin_folder\no_file_extension'
+ 1 => '/top_level_latin_folder\example_of_an_extremely_long_latin_file_name_with_some_numbers_at_the_end_0123456789.fileextension'
)
I suspect this is because I'm running the test in Windows.
Just changed it to be one single commit with two files.
Thanks @timothylcooke! Will merge this in and address the CI issues separately.
See this comment on issue #240. Can confirm this fixes the issue on our
bitnami/wordpress:6
Docker image.