Follow up from #2870448-30: Convert web tests to browser tests for file module

Problem/Motivation

Drupal\file\Tests\DownloadTest::testFileCreateUrl() contains next part:

    // Test public files with a different host name from settings.
    $test_base_url = 'http://www.example.com/cdn';
    $this->settingsSet('file_public_base_url', $test_base_url);
    $filepath = file_create_filename('test.txt', '');
    $directory_uri = 'public://' . dirname($filepath);
    file_prepare_directory($directory_uri, FILE_CREATE_DIRECTORY);
    $file = $this->createFile($filepath, NULL, 'public');
    $url = file_create_url($file->getFileUri());
    $expected_url = $test_base_url . '/' . basename($filepath);
    $this->assertEqual($url, $expected_url);

For convert this part to Functional test (phpunit), we need to add settingsSet() method too.

Proposed resolution

@dawehner posted:

This new method is used for \Drupal\file\Tests\DownloadTest::testFileCreateUrl which could be actually a kernel test, couldn't it?

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#5 interdiff-3-5.txt1.37 KBAnonymous (not verified)
#5 2919872-5.patch2.41 KBAnonymous (not verified)
#3 interdiff-2-3.txt1.35 KBAnonymous (not verified)
#3 2919872-3.patch2.83 KBAnonymous (not verified)
#2 2919872-2.patch2.83 KBAnonymous (not verified)

Comments

Anonymous’s picture

Anonymous (not verified) commented

vaplas created an issue. See original summary.

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Needs review
StatusFileSize
new 2919872-2.patch2.83 KB

Direct transfer.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 2919872-3.patch2.83 KB
new interdiff-2-3.txt1.35 KB

Clean up.

If you are a committer, and you prefer to make this transfer to #2870448: Convert web tests to browser tests for file module, just tell me about it :)
dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work 
+++ b/core/modules/file/tests/src/Kernel/FileUrlTest.php
@@ -0,0 +1,45 @@
+  /**
+   * Changes in memory settings.
+   *
+   * @param string $name
+   *   The name of the setting to return.
+   * @param string $value
+   *   The value of the setting.
+   *
+   * @see \Drupal\Core\Site\Settings::get()
+   */
+  protected function settingsSet($name, $value) {
+    $settings = Settings::getAll();
+    $settings[$name] = $value;
+    new Settings($settings);
+  }
+

Why do we not use \Drupal\KernelTests\KernelTestBase::setSetting?

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
StatusFileSize
new 2919872-5.patch2.41 KB
new interdiff-3-5.txt1.37 KB

@dawehner, thanks for review and nice hint! Done.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +phpunit initiative

This looks great for me now. Thank you!

xjm’s picture

xjm
she/her
Primary language English
commented
Title: Move part of DownloadTest::testFileCreateUrl() to Kernel test » Move part of DownloadTest::testFileCreateUrl() to a new Kernel test

The title made me think this was moving the method to KTB, which surprised me. ;)

  • xjm committed 707740e on 8.5.x 
    Issue #2919872 by vaplas, dawehner: Move part of DownloadTest::...

  • xjm committed a0b195d on 8.4.x 
    Issue #2919872 by vaplas, dawehner: Move part of DownloadTest::...
xjm’s picture

xjm
she/her
Primary language English
commented
Version: 8.5.x-dev » 8.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.5.x. Thanks! I also backported it to 8.4.x as a small test improvement.

dawehner’s picture

dawehner
Primary language German
commented

Thank you @xjm!

Status: Fixed » Closed (fixed)

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