Skip to content

Conversation

@come-nc
Copy link
Contributor

@come-nc come-nc commented Apr 5, 2022

Signed-off-by: Côme Chilliet [email protected]

@come-nc come-nc self-assigned this Apr 5, 2022
@come-nc come-nc force-pushed the enh/use-new-phpunit-setup branch from 0462843 to 4c27e7f Compare April 5, 2022 08:52
@come-nc
Copy link
Contributor Author

come-nc commented Apr 5, 2022

Testing nextcloud/.github#53

- name: PHPUnit integration
# Only run if phpunit integration config file exists
if: steps.check_integration.outputs.files_exists == 'true'
working-directory: apps/${{ env.APP_NAME }}
Copy link
Member

Choose a reason for hiding this comment

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

biggest difference is the location.
This used to get called from:

      - name: PHPUnit
        working-directory: apps/${{ env.APP_NAME }}/tests
        run: ../vendor/bin/phpunit -c phpunit.xml

This comment was marked as outdated.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure what I tested, but now I can reproduce it with this change.

activity/tests$ ../vendor/bin/phpunit -c phpunit.xml 
PHPUnit 9.5.20 #StandWithUkraine

Runtime:       PHP 7.4.28
Configuration: phpunit.xml
Warning:       No code coverage driver available
Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

...............................................................  63 / 263 ( 23%)
............................................................... 126 / 263 ( 47%)
....................................................WWWW......W 189 / 263 ( 71%)
WWWW........................................................... 252 / 263 ( 95%)
......WWWWW                                                     263 / 263 (100%)

Time: 00:05.714, Memory: 42.50 MB
activity$ ./vendor/bin/phpunit -c tests/phpunit.xml 
PHPUnit 9.5.20 #StandWithUkraine

Runtime:       PHP 7.4.28
Configuration: tests/phpunit.xml
Warning:       No code coverage driver available
Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

...............................................................  63 / 263 ( 23%)
............................................................... 126 / 263 ( 47%)
.........E..........................................WWWW......W 189 / 263 ( 71%)
WWWW........................................................... 252 / 263 ( 95%)
......WWWWW                                                     263 / 263 (100%)

Time: 00:05.679, Memory: 40.50 MB

There was 1 error:

1) OCA\Activity\Tests\Controller\ActivitiesControllerTest::testShowList
Exception: The requested uri() cannot be processed by the script './vendor/bin/phpunit')

Copy link
Member

Choose a reason for hiding this comment

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

The actual problem is here:
https://github.com/nextcloud/server/blob/eede608c0e9cd313bc672df1f0e37aca2a0ef2ad/lib/private/legacy/OC_App.php#L632

		var_dump('$request->getScriptName()', $request->getScriptName());
		var_dump('$script', $script);
		var_dump('$topFolder', $topFolder);
string(25) "$request->getScriptName()"
string(20) "./vendor/bin/phpunit"
string(7) "$script"
string(19) "/vendor/bin/phpunit"
string(10) "$topFolder"
string(0) ""

vs.

string(25) "$request->getScriptName()"
string(21) "../vendor/bin/phpunit"
string(7) "$script"
string(20) "./vendor/bin/phpunit"
string(10) "$topFolder"
string(1) "."

so in the first case
https://github.com/nextcloud/server/blob/eede608c0e9cd313bc672df1f0e37aca2a0ef2ad/lib/private/legacy/OC_App.php#L635-L636
is entered and then throws, while in the second case we continue with $topFolder = '.' ....

I guess the function needs some special handling for CLI cases or at least should not throw....

Copy link
Member

Choose a reason for hiding this comment

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

@come-nc
Copy link
Contributor Author

come-nc commented Jun 13, 2022

/rebase

come-nc and others added 2 commits June 13, 2022 12:27
Signed-off-by: Joas Schilling <[email protected]>
@nextcloud-command nextcloud-command force-pushed the enh/use-new-phpunit-setup branch from ac4fed2 to 57c01a2 Compare June 13, 2022 12:27
Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Adjusted required job list

@nickvergessen nickvergessen merged commit 8e8fc6f into master Jun 13, 2022
@delete-merged-branch delete-merged-branch bot deleted the enh/use-new-phpunit-setup branch June 13, 2022 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants