Problem/Motivation

It would be great to see this module tested with a "real" Drupal 7 migration.

Proposed resolution

Create a Drupal 7 migration fixture (with DB and files) and (at least) a kernel test that tests the expected outcome.

Remaining tasks

Must-have's:

  1. Create a Drupal 7 migration fixture. Done.
  2. Create a kernel test. Done.
  3. Enable automated tests for this project. I think that certain DB queries will fail with PostreSQL, so the initial test env(s) should use MySQL or SQLite.

Could-have's:

  1. The above for Drupal 6 → Drupal 8|9 migration
  2. A functional test that uses core's Migrate Drupal UI module
  3. A functional test that uses Migrate Upgrade and Drush

Additional info

The test will pass if you apply the following patches (sorted list):

  1. #3164697-3: migrations/d7 directory
  2. #3170195-3: Migrate conditions of type "contains"
  3. #3113558: Submission data values are not saved This is not required anymore.
  4. #3172154-10: Fix source plugin's undefined index issues, schema issues and unnecessary trailing whitespaces and empty lines
  5. #3172161-2: Fix the migration of option selects with groups
  6. #3172162-2: Adjust (the correct) value for webform submission's URI
  7. #3172176-2: D7 Fix the migration of nested values, multi-value field types (e.g. checkboxes) and handle 'grid' type submission values
  8. #3172417-5: [PP-1] d7_webform_submissions migrates uid without using migration lookup
CommentFileSizeAuthor
#32 webform_migrate-kernel_test-3172139-32.patch2.69 KBjienckebd
#21 webform_migrate-kernel_test-3172139-19.diff855.48 KBhuzooka
#19 webform_migrate-kernel_test-3172139-19.patch.patch875.54 KBMatroskeen
#14 webform_migrate-kernel_test-3172139-14.patch871.52 KBMatroskeen
#12 webform_migrate-kernel_test-3172139-12.patch854.75 KBMatroskeen
#5 interdiff-3172139-4-5.txt1.04 KBhuzooka
#5 webform_migrate-kernel_test-3172139-5.patch884.37 KBhuzooka
#4 webform_migrate-kernel_test-3172139-4.patch884.16 KBhuzooka
#2 webform_migrate-kernel_test-3172139-2.patch883.58 KBhuzooka

Issue fork webform_migrate-3172139

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

huzooka created an issue. See original summary.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented
Assigned: huzooka » Unassigned
Status: Active » Needs review
FileSize
webform_migrate-kernel_test-3172139-2.patch883.58 KB
huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented
Assigned: Unassigned » huzooka
Status: Needs review » Needs work
huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented
Assigned: huzooka » Unassigned
Issue summary: View changes
Status: Needs work » Needs review
FileSize
webform_migrate-kernel_test-3172139-4.patch884.16 KB
huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented
FileSize
webform_migrate-kernel_test-3172139-5.patch884.37 KB
interdiff-3172139-4-5.txt1.04 KB
huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented
Issue summary: View changes
huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented
Assigned: Unassigned » huzooka
Status: Needs review » Needs work

I just have updated #3172154: Fix source plugin's undefined index issues, schema issues and unnecessary trailing whitespaces and empty lines, and now I have to update the test.

huzooka opened merge request !5

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented
Assigned: huzooka » Unassigned
Status: Needs work » Needs review
huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented
Issue summary: View changes

Updated the patch list.

Matroskeen made their first commit to this issue’s fork.

Matroskeen’s picture

Matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
CreditAttribution: Matroskeen at DevBranch, Evolving Web, Drupal Ukraine Community for University of Waterloo commented
FileSize
webform_migrate-kernel_test-3172139-12.patch854.75 KB

Even if the annotation is there, it still complains about missing webform module 🤔
Maybe there is something wrong with MR flow, trying with a patch.

The last submitted patch, 5: webform_migrate-kernel_test-3172139-5.patch, failed testing. View results

Matroskeen’s picture

Matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
CreditAttribution: Matroskeen at DevBranch, Evolving Web, Drupal Ukraine Community for University of Waterloo commented
FileSize
webform_migrate-kernel_test-3172139-14.patch871.52 KB

trying with --binary flag

Status: Needs review » Needs work

The last submitted patch, 14: webform_migrate-kernel_test-3172139-14.patch, failed testing. View results

andriyun’s picture

andriyun CreditAttribution: andriyun as a volunteer and at Bellcom, Drupal Ukraine Community for Drupal Ukraine Community commented
Issue summary: View changes

Automated tests enabled

andriyun’s picture

andriyun CreditAttribution: andriyun as a volunteer and at Bellcom, Drupal Ukraine Community for Drupal Ukraine Community commented

1.

+++ b/tests/src/Traits/WebformMigrateAssertionsTrait.php
@@ -0,0 +1,845 @@
+    '#type': time

Type for time werbform element should be `webform_time`

2. Missing '#step' attribute
it's present in webform source `"minuteincrements";s:1`

andriyun’s picture

andriyun CreditAttribution: andriyun as a volunteer and at Bellcom, Drupal Ukraine Community for Drupal Ukraine Community commented

Adjusted assert

Matroskeen’s picture

Matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
CreditAttribution: Matroskeen at DevBranch, Evolving Web, Drupal Ukraine Community for University of Waterloo commented
Status: Needs work » Needs review
FileSize
webform_migrate-kernel_test-3172139-19.patch.patch875.54 KB

For some reason, it doesn't fetch external dependencies with the merge request flow. Re-uploading the MR content as a patch.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented

I miss the interdiff, but 🥳!

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented
FileSize
webform_migrate-kernel_test-3172139-19.diff855.48 KB

I'm using the files in this patch/mr for doing some integration testing.

This is a diff which is identical to #19, but makes anyone able to apply it on top of the distributed release of Webform Migrate.

  • andr1yun committed adc1900 on 8.x-1.x authored by huzooka 
    Issue #3172139 by huzooka, Matroskeen, andr1yun: Enable project testing...
andriyun’s picture

andriyun CreditAttribution: andriyun as a volunteer and at Bellcom, Drupal Ukraine Community for Drupal Ukraine Community commented
Status: Needs review » Fixed

I've added commit with changes in #19
The rest of work moved out to followup issue #3206977: Drupal 6 - Add kernel tests, source database, files

Thanks

andr1yun closed merge request !5

andr1yun opened merge request !7

andriyun’s picture

andriyun CreditAttribution: andriyun as a volunteer and at Bellcom, Drupal Ukraine Community for Drupal Ukraine Community commented
Status: Fixed » Needs review

@huzooka
I’ve managed to get tests passed when they run by Drupal CI from merge request.

There were missing some dependencies in composer.json
And default values where treated strange. So I changed them to custom values.

Please review changes in MR7 https://git.drupalcode.org/project/webform_migrate/-/merge_requests/7/di...

Matroskeen’s picture

Matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
CreditAttribution: Matroskeen at DevBranch, Evolving Web, Drupal Ukraine Community for University of Waterloo commented

@andr1yun asked me to take a look, so I added my thoughts to the merge request.
It seems like a nice workaround, but it looks like a bug in the test runner that would be great to fix. I asked in #drupal-infrastructure Slack channel to verify.

huzooka’s picture

huzooka
Primary language Hungarian
Location Hungary 🇭🇺🇪🇺
CreditAttribution: huzooka at Cheppers for Acquia commented

You can also execute the migrations with Migrate Drupal UI, so afaik Migrate Tools must not be a dependency.

Matroskeen’s picture

Matroskeen
Primary language Ukrainian
Location 🇺🇦 Ukraine, Lutsk
CreditAttribution: Matroskeen at DevBranch, Evolving Web, Drupal Ukraine Community for University of Waterloo commented

It was confirmed in #drupal-infrastucture that dependencies should be specified in composer.json to work properly with merge requests, so we're going into the right direction.

See: https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or...

Liam Morland made their first commit to this issue’s fork.

jienckebd’s picture

jienckebd CreditAttribution: jienckebd as a volunteer commented
Version: 8.x-1.x-dev » 2.0.x-dev
jienckebd’s picture

jienckebd CreditAttribution: jienckebd as a volunteer and at Acquia commented
FileSize
webform_migrate-kernel_test-3172139-32.patch2.69 KB

It seems that the kernel tests are failing because of schema changes in Webform 6.

The attached patch helps these tests pass.