From 421b4399103a41c740f1cb0630b6ee6f79637a79 Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Wed, 2 Aug 2023 13:45:01 +1000 Subject: [PATCH 1/2] test: avoid using class variables in this test to avoid mem leaks Per Totara unit test feedback/failures. --- tests/tool_dataflows_reader_csv_test.php | 44 ++++++++++-------------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/tests/tool_dataflows_reader_csv_test.php b/tests/tool_dataflows_reader_csv_test.php index 131c1faf..7d2791f9 100644 --- a/tests/tool_dataflows_reader_csv_test.php +++ b/tests/tool_dataflows_reader_csv_test.php @@ -36,20 +36,12 @@ */ class tool_dataflows_reader_csv_test extends \advanced_testcase { - /** @var string|null Input path for reader to read from. */ - protected $inputpath = null; - - /** @var string Temp path for test files. */ - protected $outputpath = ''; - /** * Set up before each test */ protected function setUp(): void { parent::setUp(); $this->resetAfterTest(); - - $this->inputpath = null; set_config('permitted_dirs', '/tmp', 'tool_dataflows'); } @@ -57,12 +49,12 @@ protected function setUp(): void { * Test csv with headers included in the file contents */ public function test_csv_with_headers_included_in_the_file_contents() { - [$dataflow, $steps] = $this->create_dataflow(); + [$dataflow, $steps, $inputpath, $outputpath] = $this->create_dataflow(); $reader = $steps[$dataflow->steps->reader->id]; $reader->vars = Yaml::dump(['testapple' => '${{ record.apple }}']); $reader->config = Yaml::dump([ - 'path' => $this->inputpath, + 'path' => $inputpath, 'headers' => '', 'delimiter' => ',', ]); @@ -78,7 +70,7 @@ public function test_csv_with_headers_included_in_the_file_contents() { $content .= implode(',', $row); $content .= PHP_EOL; } - file_put_contents($this->inputpath, $content); + file_put_contents($inputpath, $content); // Execute. ob_start(); @@ -86,7 +78,7 @@ public function test_csv_with_headers_included_in_the_file_contents() { $engine->execute(); ob_get_clean(); - $output = file_get_contents($this->outputpath); + $output = file_get_contents($outputpath); $this->assertEquals(7, $engine->get_variables_root()->get('steps.reader.vars.testapple')); $this->assertEquals($content, $output); @@ -96,13 +88,13 @@ public function test_csv_with_headers_included_in_the_file_contents() { * Test csv with custom headers configured */ public function test_csv_with_custom_headers_configured() { - [$dataflow, $steps] = $this->create_dataflow(); + [$dataflow, $steps, $inputpath, $outputpath] = $this->create_dataflow(); $headers = ['first', 'second', 'third']; $reader = $steps[$dataflow->steps->reader->id]; $reader->vars = Yaml::dump(['testsecond' => '${{ record.second }}']); $reader->config = Yaml::dump([ - 'path' => $this->inputpath, + 'path' => $inputpath, 'headers' => implode(',', $headers), 'delimiter' => ',', ]); @@ -118,7 +110,7 @@ public function test_csv_with_custom_headers_configured() { $content .= implode(',', $row); $content .= PHP_EOL; } - file_put_contents($this->inputpath, $content); + file_put_contents($inputpath, $content); // Execute. ob_start(); @@ -126,7 +118,7 @@ public function test_csv_with_custom_headers_configured() { $engine->execute(); ob_get_clean(); - $output = file_get_contents($this->outputpath); + $output = file_get_contents($outputpath); $this->assertEquals(8, $engine->get_variables_root()->get('steps.reader.vars.testsecond')); // Expected output; Add a header line. @@ -138,7 +130,7 @@ public function test_csv_with_custom_headers_configured() { * Tests for an invalid input stream. */ public function test_bad_input_stream() { - [$dataflow, $steps] = $this->create_dataflow(); + [$dataflow, $steps, $inputpath, $outputpath] = $this->create_dataflow(); $path = 'path/to/nowhere'; $reader = $steps[$dataflow->steps->reader->id]; @@ -166,12 +158,12 @@ public function test_bad_input_stream() { * Test csv with custom headers configured */ public function test_csv_with_overwrite_headers_configured() { - [$dataflow, $steps] = $this->create_dataflow(); + [$dataflow, $steps, $inputpath, $outputpath] = $this->create_dataflow(); $headers = ['first', 'second', 'third']; $reader = $steps[$dataflow->steps->reader->id]; $reader->config = Yaml::dump([ - 'path' => $this->inputpath, + 'path' => $inputpath, 'headers' => implode(',', $headers), 'overwriteheaders' => '1', 'delimiter' => ',', @@ -188,7 +180,7 @@ public function test_csv_with_overwrite_headers_configured() { $content .= implode(',', $row); $content .= PHP_EOL; } - file_put_contents($this->inputpath, $content); + file_put_contents($inputpath, $content); // Expected content, which has headers but first line of content is removed. array_shift($data); @@ -204,7 +196,7 @@ public function test_csv_with_overwrite_headers_configured() { $engine->execute(); ob_get_clean(); - $output = file_get_contents($this->outputpath); + $output = file_get_contents($outputpath); // Expected output; Add a header line. $this->assertEquals($expected, $output); @@ -224,25 +216,25 @@ public function create_dataflow(): array { $steps = []; - $this->inputpath = tempnam('', 'tool_dataflows'); + $inputpath = tempnam('', 'tool_dataflows'); $reader = new step(); $reader->name = 'reader'; $reader->type = reader_csv::class; $reader->config = Yaml::dump([ - 'path' => $this->inputpath, + 'path' => $inputpath, 'headers' => '', 'delimiter' => ',', ]); $dataflow->add_step($reader); $steps[$reader->id] = $reader; - $this->outputpath = tempnam('', 'tool_dataflows'); + $outputpath = tempnam('', 'tool_dataflows'); $writer = new step(); $writer->name = 'stream-writer'; $writer->type = 'tool_dataflows\local\step\writer_stream'; $writer->config = Yaml::dump([ 'format' => 'csv', - 'streamname' => $this->outputpath, + 'streamname' => $outputpath, ]); $writer->depends_on([$reader]); $dataflow->add_step($writer); @@ -250,6 +242,6 @@ public function create_dataflow(): array { $this->reader = $reader; $this->writer = $writer; - return [$dataflow, $steps]; + return [$dataflow, $steps, $inputpath, $outputpath]; } } From 3b63c2490805a1bd73144d22fbafb927553b9c0f Mon Sep 17 00:00:00 2001 From: Kevin Pham Date: Wed, 2 Aug 2023 14:18:32 +1000 Subject: [PATCH 2/2] test: use https mock url by default Resolves 301 redirect handling issue in one of the test --- tests/application_trait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/application_trait.php b/tests/application_trait.php index 83e41f13..24934d48 100644 --- a/tests/application_trait.php +++ b/tests/application_trait.php @@ -44,7 +44,7 @@ trait application_trait { * @return string mock url to use */ public function get_mock_url(string $path): string { - return 'http://download.moodle.org/unittest'.$path; + return 'https://download.moodle.org/unittest'.$path; } // @codingStandardsIgnoreStart