- Issue created by @manuel garcia
- π¬π§United Kingdom oily Greater London
andrew.farquharson β made their first commit to this issueβs fork.
- Merge request !6change core compatibility from ^8 || ^9 to ^9 || ^10 in test module β (Closed) created by oily
- π¬π§United Kingdom oily Greater London
- First i copied the gitlab-ci.yml file created in issue #3406407 and committed it to this issue branch
- I then fixed the test module compatibility with Drupal 10
- That got the PHPUnit tests to complete successfully.
- There were a few warnings in the eslint and php code sniffer tests that I fixed in further commits.
- All the tests then ran successfully
Not sure about the best ticket management but i think that if this passes review it could close issue #3406407 too.
- Status changed to Needs review
over 1 year ago 11:43pm 7 December 2023 - Status changed to Needs work
over 1 year ago 11:34am 8 December 2023 - πͺπΈSpain manuel garcia
thanks Andrew for the work on this, its nice to see the tests running on Gitlab CI!
On this ticket we will be committing only whats needed for Drupal 10 compatibility. Once we fix this ticket we will tackle getting the Gtlab CI
running on β¨ Configure GitLab CI PostponedRegarding the error fixed by adding of the permission configuration (https://git.drupalcode.org/issue/view_profiles_perms-3406820/-/jobs/452805), I think that adding the config file is not the correct solution, as this a permission dynamically provided by view_profiles_perms module, which is what we want to test for. However lead me to investigate further, and I eventually landed on #2571235: [regression] Roles should depend on objects that are building the granted permissions β - see its change records:
So this may technically be a actual current bug in Drupal 10? Not sure to be honest, but we probably have to mimic how
\Drupal\taxonomy\TaxonomyPermissions
does things - Open on Drupal.org βCore: 10.1.x + Environment: PHP 8.2 & MySQL 8last update
over 1 year ago Waiting for branch to pass - last update
over 1 year ago 1 pass - Status changed to Needs review
over 1 year ago 9:21am 9 December 2023 - πͺπΈSpain manuel garcia
Added PHPstan config file on https://git.drupalcode.org/project/view_profiles_perms/-/merge_requests/... to handle warnings https://git.drupalcode.org/project/view_profiles_perms/-/jobs/460383
$ php vendor/bin/phpstan analyze $_WEB_ROOT/modules/custom/$CI_PROJECT_NAME $PHPSTAN_CONFIGURATION --no-progress || EXIT_CODE=$? ------ ------------------------------------------------------------------------------ Line src/ViewProfilesPermsPermissions.php ------ ------------------------------------------------------------------------------ 41 Unsafe usage of new static(). π‘ See: https://phpstan.org/blog/solving-phpstan-error-unsafe-usage-of-new-static ------ ------------------------------------------------------------------------------
https://www.drupal.org/docs/develop/development-tools/phpstan/handling-u... β for context.
- π¬π§United Kingdom oily Greater London
Hi @Manuel Garcia, Thank you for fixing the code! I am pleased that you found the source of the test error: I was struggling with the test. I will review the PHPStan config soon.
- π¬π§United Kingdom oily Greater London
@Manuel Garcia, I have reviewed your most recent commits and I have inspected the output of the pipelines after your commits. I see that you have fixed the tests and all the assertions in the functional test completed. The test is fixed and the assertions are passing
I have not yet fully reviewed the change to the pipeline configuration. But i agree that the configuration needs to be tuned and I believe that the relaxation rule on the static functions should be relaxed to suppress warnings.
Since the test is now working properly I believe this issue is now fixed. But if there are more tasks to perform please I will try to help.
- π¬π§United Kingdom oily Greater London
Using setup to run gitlab tests locally I have run the PHPUnit tests on this issue branch:
ddev phpunit
. It reportsOK (1 test, 29 assertions)
- Open on Drupal.org βCore: 10.1.x + Environment: PHP 8.2 & MySQL 8last update
over 1 year ago Waiting for branch to pass - last update
about 1 year ago 1 pass - πͺπΈSpain manuel garcia
Ok I have compiled the changes form the PR #6 into the attached patch, except the changes related to gitlabci which we will add on β¨ Configure GitLab CI Postponed .
-
Manuel Garcia β
committed 17110c48 on 2.1.x
Issue #3406820 by andrew.farquharson, Manuel Garcia:...
-
Manuel Garcia β
committed 17110c48 on 2.1.x
- Status changed to Fixed
about 1 year ago 8:50am 11 December 2023 - πͺπΈSpain manuel garcia
OK patch came back green, committed and pushed to 2.1.x - thanks!
Automatically closed - issue fixed for 2 weeks with no activity.