Skip to content

Commit

Permalink
Add: Include "test" folder to gitattribute's exclusion (#368)
Browse files Browse the repository at this point in the history
  • Loading branch information
connorhu authored Apr 4, 2024
1 parent 0c9ba11 commit 3f0adca
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@
docker-compose.yml export-ignore
phpstan.neon export-ignore
phpunit.xml export-ignore
test/ export-ignore
tests/ export-ignore

12 comments on commit 3f0adca

@JohannesTyra
Copy link

@JohannesTyra JohannesTyra commented on 3f0adca Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@connorhu with this change the autoload will fail, because this files are hardcoded in sfCoreAutoload.class.php
vendor/friendsofsymfony1/symfony1/lib/autoload/sfCoreAutoload.class.php

    'sflimeharness' => 'task/test/sfLimeHarness.class.php',
    'sftestalltask' => 'task/test/sfTestAllTask.class.php',
    'sftestbasetask' => 'task/test/sfTestBaseTask.class.php',
    'sftestcoveragetask' => 'task/test/sfTestCoverageTask.class.php',
    'sftestfunctionaltask' => 'task/test/sfTestFunctionalTask.class.php',
    'sftestplugintask' => 'task/test/sfTestPluginTask.class.php',
    'sftestunittask' => 'task/test/sfTestUnitTask.class.php',

@JohannesTyra
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its:
php symfony PHP Warning: require(/xxxx/vendor/friendsofsymfony1/symfony1/lib/task/test/sfTestPluginTask.class.php): Failed to open stream: No such file or directory in /xxxx/vendor/friendsofsymfony1/symfony1/lib/autoload/sfCoreAutoload.class.php on line 451 PHP Fatal error: Uncaught Error: Failed opening required '/xxxx/vendor/friendsofsymfony1/symfony1/lib/task/test/sfTestPluginTask.class.php' (include_path='.:/usr/share/php') in /xxxx/vendor/friendsofsymfony1/symfony1/lib/autoload/sfCoreAutoload.class.php:451 Stack trace: #0 [internal function]: sfCoreAutoload->autoload()

@iricketson
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thePanz @alquerci This change should be reverted, as it removes the test directory from the symfony1/lib dir.

Internal error: Failed opening required '/path/to/friendsofsymfony1/symfony1/lib/test/sfTestFunctional.class.php'

@alquerci
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @iricketson

The revert is already done, at least fixes the issue you exposed.

7afe4d2

@iricketson
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alquerci @thePanz Can we tag 1.5.21 due to breaking change?

@thePanz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iricketson I will give it a try today to tag a new release.

How did you use SF1 to trigger that error? @JohannesTyra @iricketson
See my comment here: #379 (comment)

do you use plain SF1 or wrap it in a newer SF version?

@JohannesTyra
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thePanz I use plain SF1 and while php symfony cc or other tasks, the errors occurs.

@iricketson
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thePanz Running phpstan caught it after updating the dependencies. Not wrapped.

My guess is it is trying to evaluate lime tests that instantiate symfony functional testing, like so:

$browser = new sfTestFunctional(new sfBrowser());

@thePanz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iricketson @JohannesTyra I tagged v1.5.21, let me know if this helps to solve your issues

thanks for the clarifications!

it would be interesting to include phpstan to test the project, maybe those issues could have been identified earlier?

@iricketson
Copy link
Contributor

@iricketson iricketson commented on 3f0adca Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thePanz I'm not sure it would have caught it when just testing the project, because just cloning the repo will not exclude the test dir. The issue is that composer evaluates .gitattributes when installing dependencies and removes files/directories listed as export-ignore when installed as a dependency of another project.

@thePanz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iricketson yeah, I am aware of the way composer installs libs from Git repositories (via the git archive)

Maybe a solution would be to have a pipeline running composer inst on a dummy project, and testing it there 🤔

@JohannesTyra
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thePanz its working! The files I mentioned (and are needed) are back again ;)

Please sign in to comment.