Goal: Remediate this module to prepare for D9.
Requirements:
1. Assign yourself the module.
2. Review the module for D9 readiness using status upgrade module or Drupal check
3. Note issues in the report
4. Remediate them and report back to this ticket
5. Resolve when all deprecations are completed
------ ---------------------------------------------------------------------------------------------------------------------------------------
Line modules/video_embed_media/modules/vem_migrate_oembed/tests/src/Functional/oEmbedUpdateTest.php
------ ---------------------------------------------------------------------------------------------------------------------------------------
Class Drupal\Tests\vem_embed_media\Functional\oEmbedUpdateTest was not found while trying to analyse it - autoloading is probably not
configured properly.
------ ---------------------------------------------------------------------------------------------------------------------------------------
------ ---------------------------------------------------------------------
Line modules/video_embed_media/video_embed_media.install
------ ---------------------------------------------------------------------
21 Call to deprecated function file_scan_directory():
in drupal:8.8.0 and is removed from drupal:9.0.0.
Use \Drupal\Core\File\FileSystemInterface::scanDirectory() instead.
------ ---------------------------------------------------------------------
Comment | File | Size | Author |
---|---|---|---|
#30 | 3097287-30.patch | 6.68 KB | phenaproxima |
| |||
#28 | 3097287-28.patch | 6.5 KB | phenaproxima |
#27 | 3097287-27.patch | 6.57 KB | phenaproxima |
| |||
#25 | 3097287-25.patch | 6.02 KB | phenaproxima |
#23 | 3097287-23.patch | 5.32 KB | phenaproxima |
Comments
Comment #2
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commentedComment #3
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commentedComment #4
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commentedUpdating patch for depreciation.: Call to deprecated function file_scan_directory():
for Class Drupal\Tests\vem_embed_media\Functional\oEmbedUpdateTest was not found while trying to analyse it - autoloading is probably not
configured properly.
this seems to be an autoload issue. Not able to fix it.
Comment #5
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commentedfixing bug in patch.
Comment #6
Sharique CreditAttribution: Sharique as a volunteer and at Acquia commentedUse filesystem service instead, of interface.
Comment #7
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commentedThanks, @sharique uploading new patch as per #6
Comment #8
joshi.rohit100Patch looks fine and applied cleanly. However, tests are failing and seems tests are failing because of colorbox related.
@saurabh.tripathi.cs can u pls check if these failures are specific to patch (which to me looks not) by running tests with and without patch?
Comment #9
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commented@joshi.rohit100 ., Yes I am getting the same issue with or without patch.
Comment #10
joshi.rohit100@saurabh.tripathi.cs - OK! Then test failing looks not related to the patch.
Please create a new issue for failing tests.
Please add "core_version_requirement" in info.yml and composer.json file.
Comment #11
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commentedHave created an issue for failing tests : here
Adding patch for changes in info file and composer.json.
Comment #12
joshi.rohit100@saurabh.tripathi.cs earlier, there were only 5 tests failing but with latest patch, its 7 now. So something is not correct. Pls check.
Also,
This is available only in 8.8 version and thus this module won't be usable for other drupal8 version.
Please do something like
Comment #13
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commentedAll the test related issues are failing due to unmet test dependencies
Still looking into it, have uploaded patch for fix suggested in #12
Comment #14
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commentedComment #15
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commentedHave edited root path for module, checking if this fixes the issue.
Comment #16
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commentedFixing coding standards. And reuploading patch with a relative path, as adding absolute path is causing the patch to not get applied.
Comment #17
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commentedFixing the variable assignment.
Comment #18
saurabh.tripathi.cs CreditAttribution: saurabh.tripathi.cs as a volunteer and at Acquia for Acquia commented@rohit These tests are failing in other issues as well
for example: https://www.drupal.org/project/video_embed_field/issues/3086712
And these are unrelated, and we can merge this patch.
\
Video_embed_field.Drupal\Tests\video_embed_field\Kernel\ConstraintTest
Video_embed_field.Drupal\Tests\video_embed_field\Kernel\FieldOutputTest
Video_embed_field.Drupal\Tests\video_embed_field\Kernel\FormatterDependenciesTest
Drupal\Tests\video_embed_field\Kernel\VideoEmbedIFrameTest
" Video_embed_wysiwyg.Drupal\Tests\video_embed_wysiwyg\Kernel\FilterTest"
Extra tests which are failing in this issue, but are also unrelated.
ideo_embed_field.Drupal\Tests\video_embed_field\FunctionalJavascript\ColorboxFormatterTest
"
Drupal\Tests\video_embed_field\Functional\FormatterConfigurationTest"
Have already created an issue for failing tests: here
Comment #19
joshi.rohit100this should be double pipe not single. See https://www.drupal.org/node/3070687
this should be
^8.7 || ^9
Comment #20
joshi.rohit100Test failing looks not related to this.
Also, this patch can not be merged as the only required change regarding file system is in D-8.8 and thus this can only be merged and required only for D-8.8 version.
Comment #21
amitgoyal CreditAttribution: amitgoyal at Acquia commented#20 looks good to me.
Comment #22
phenaproximaNot quite...there are still a couple of calls to entity_get_form_display() in a test. This patch removes them.
Comment #23
phenaproximaMerged #3117181: Declare Drupal 9 compatibility into this one, so it will only take one commit to get this module D9 ready.
Comment #25
phenaproximaMade a couple of small changes which I hope will help fix most, if not all, of the failures.
Comment #27
phenaproximaThis should hopefully do better.
Comment #28
phenaproximaI just realized that my approach to the Colorbox info file is potentially destructive if Colorbox is actually installed. Seeing as how it is already explicitly declared as a test dependency, I don't think we need to "fake-enable" Colorbox in the kernel tests; let's enable it for real.
Comment #30
phenaproximaHow 'bout this...
Comment #32
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedNice, thanks.