The existing test for image insert into a WYSIWYG field fails if the private file path is not set. This test always passes in Panopoly CI because setting the private file path is part of the Travis build. But it's not guaranteed to be set in other circumstances.

To fix, we could do this:

  1. Before suite, check if private file path is set
  2. If not, generate a random string and set the private path to a folder by that name under the public directory
  3. Run test and pass
  4. After suite, check if private file path matches random string from before
  5. If so, unset it.
CommentFileSizeAuthor
#13 panopoly_test-private-files-2268641-13.patch3.66 KBcboyden
#6 panopoly_test-private-files-2268641-6.patch3.67 KBdsnopek
#2 panopoly_test-private-files-2268641-2.patch3.65 KBdsnopek

Comments

cboyden’s picture

cboyden commented
Issue summary: View changes
dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Active » Needs review
StatusFileSize
new panopoly_test-private-files-2268641-2.patch3.65 KB

Here's the first pass at a patch to implement this. I've tested it locally on a site without a private path setup and it worked for me! I'm also going to try on Travis-CI in a moment.

@cboyden: Can you try this patch in your environment?

cboyden’s picture

cboyden commented

I'm getting an exception on a fresh install of latest Panopoly:

"Files must be configured for @drupal_private_files tests to work!"

When I go to admin/config/media/file-system, I see sites/default/files in the "Public file system path" field. But, if this form has never been saved in the UI, it isn't in the variables table. So variable_get can't find it and thinks it's empty.

cboyden’s picture

cboyden commented
Status: Needs review » Needs work
dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

Blergh! I always configure my files paths with settings.php (which is how my local dev site is setup too), so I didn't take into account a site that wouldn't have that setup BUT still have a working files configuration.

I'll find where it's calculating it's default value and pull that code in too...

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Needs work » Needs review
StatusFileSize
new panopoly_test-private-files-2268641-6.patch3.67 KB

The fix was super simple - just pulled in the same default from system.admin.inc. Getting it properly tested, however, took a little bit. :-)

Try this patch!

cboyden’s picture

cboyden commented

Cool! One more question: Can the private file path be set by a method that does not make it appear in the variables table? If so, we'd have to add the same check for that as you've just added for the public file path.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

One more question: Can the private file path be set by a method that does not make it appear in the variables table?

Hrm. The only other way would be overwritting the site's settings.php, which we really shouldn't have permission to do in any sane environment. :-)

If so, we'd have to add the same check for that as you've just added for the public file path.

I'm not sure what you mean. I didn't add a check, just a default. And the private path doesn't have a default, since Drupal can't guess a path that isn't accessible to the webserver.

cboyden’s picture

cboyden commented

This test passes on Chrome (it's already tagged @chrome, so no surprise that it does not pass on Firefox).

It works under both conditions - variable set or unset - and unsets the variable correctly after the test if it started off unset.

The point of my previous question was, is there any chance that the site under test has already set the private file directory in a way that does not show up in the variables table? Because if it does, the test will wipe that setting out.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

The point of my previous question was, is there any chance that the site under test has already set the private file directory in a way that does not show up in the variables table? Because if it does, the test will wipe that setting out.

Oooh. No, I don't think so. There is no default for file_private_path, it has to get setup manually. And if the user used the settings.php to set file_private_path (or manipulated $conf manually), it would actually override what's on the variables table, not the other way around.

I think we're fine here. That is, until someone reports a problem with a case I didn't think of. :-) But, yeah, I can't think of anything that could go wrong at the moment.

  • dsnopek committed 6564158 on 7.x-1.x 
    Update Panopoly Test for #2268641 by dsnopek | cboyden: Fixed WYSIWYG...
dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Needs review » Fixed

Committed!

cboyden’s picture

cboyden commented
StatusFileSize
new panopoly_test-private-files-2268641-13.patch3.66 KB

For anyone following along at home: the commit ended up being a bit different. I've attached a patch that matches the commit.

Status: Fixed » Closed (fixed)

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