In order to ensure the stability of the module after fixing issues it would be helpful to have some automated tests. These could cover the top level logic ensuring that the core tasks work and that they do not throw any warnings.

A patch will be posted in the next comment.

CommentFileSizeAuthor
#12 devel-test-devel-generate-1751084-12.patch4.22 KBjuampynr
#10 devel-test-devel-generate-1751084-10.patch4.2 KBjuampynr
#8 devel-test-devel-generate-1751084-8.patch4.08 KBjuampynr
#4 devel-test-devel-generate-1751084-4-do-not-test.patch4.08 KBjuampynr
#1 devel-test-devel-generate-1751084-1.patch3.38 KBjuampynr

Comments

juampynr’s picture

juampynr commented
Status: Active » Needs review
StatusFileSize
new devel-test-devel-generate-1751084-1.patch3.38 KB

The attached patch adds tests for generate-vocabs, generate-terms and generate-menus commands.

Status: Needs review » Needs work

The last submitted patch, devel-test-devel-generate-1751084-1.patch, failed testing.

juampynr’s picture

juampynr commented
Status: Needs work » Active

The reason why the test fails is because there is a wrong statement at devel_generate calling taxonomy_vocabulary_load_multiple(), which changed its signature in Drupal 8. This has been fixed at #1748610: generate-x commands are not formatting correctly status messages.

juampynr’s picture

juampynr commented
Status: Active » Needs review
StatusFileSize
new devel-test-devel-generate-1751084-4-do-not-test.patch4.08 KB

Here is a new version of the patch. It now covers generate-content too.

There is still the warning mentioned at #3 and also a weird behaviour when activating the Delete old content checkbox when generating content. Apart from it it does cover the main interactions of these features.

salvis’s picture

salvis
he/his
commented
Status: Needs review » Postponed

Thank you for taking this on — getting real tests is great!

Moshe will have to review this, but he'll probably wait until someone else has RTBC'd it.

In the meantime let me say this:

The reason why the test fails is because there is a wrong statement at devel_generate calling taxonomy_vocabulary_load_multiple(), which changed its signature in Drupal 8. This has been fixed at #1748610: generate-x commands are not formatting correctly the generated data.

It's unfortunate that you structure your patches this way and it may delay testing and committing.

If you find a porting bug that causes something to break then you should provide a small focused patch for that (which would have good chances of being reviewed and committed quickly), rather than burying it in a much larger patch like #1748610: generate-x commands are not formatting correctly status messages that addresses a cosmetic issue.

Postponing this because it cannot be reviewed before the other patch is committed.

salvis’s picture

salvis
he/his
commented

BTW, setting the patch to do-not-test is not the solution. I can't speak for Moshe, but personally I don't open anything that's not green, and because you named it do-not-test we can't even re-test #4. You'll have to re-upload it once the prerequisite is committed.

juampynr’s picture

juampynr commented

Created a separate issue to fix the related warning. See #1753830: Wrong parameter on taxonomy_vocabulary_load_multiple().

juampynr’s picture

juampynr commented
StatusFileSize
new devel-test-devel-generate-1751084-8.patch4.08 KB

Here is the patch again without the do-not-test postfix. Please, once #1753830: Wrong parameter on taxonomy_vocabulary_load_multiple() gets committed re-run tests against this patch so they pass.

moshe weitzman’s picture

moshe weitzman commented
Status: Postponed » Needs work

Thanks ... I'm seeing failures in taxonomy and generate content even after committing #1753830: Wrong parameter on taxonomy_vocabulary_load_multiple().

juampynr’s picture

juampynr commented
Status: Needs work » Needs review
StatusFileSize
new devel-test-devel-generate-1751084-10.patch4.2 KB

Errors fixed.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Fixed

My D8 won't run tests for some reason but I committed it anyway. Hope it passes.

Feel free to reopen for D7

juampynr’s picture

juampynr commented
Version: 8.x-1.x-dev » 7.x-1.x-dev
Status: Fixed » Needs review
StatusFileSize
new devel-test-devel-generate-1751084-12.patch4.22 KB

Here it is for Drupal 7.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Fixed

Committed to 7.x. Thanks.

It would be interesting to see if this could be done as a pure unit test, instead of a web test, at least for D8. Not urgent.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.