issue in phpunit

[baselbj]

Hi,

I am trying to upload my new pull request and the test create the following error which I found on other pull requests and I think the issue is not in my edits:

1. CodeIgniter\HTTP\numberHelperTest::test_number_to_size
   TypeError: Return value of CodeIgniter\Language\Language::requireFile() must be of the type array, boolean returned
   /home/travis/build/bcit-ci/CodeIgniter4/system/Language/Language.php:213
   /home/travis/build/bcit-ci/CodeIgniter4/system/Language/Language.php:176
   /home/travis/build/bcit-ci/CodeIgniter4/system/Language/Language.php:140
   /home/travis/build/bcit-ci/CodeIgniter4/system/Language/Language.php:102
   /home/travis/build/bcit-ci/CodeIgniter4/system/Common.php:379
   /home/travis/build/bcit-ci/CodeIgniter4/system/Helpers/number_helper.php:82
   /home/travis/build/bcit-ci/CodeIgniter4/tests/system/Helpers/NumberHelperTest.php:27

[kenjis]

I can't reproduce it. Does someone have a clue?

[SmolSoftBoi]

I'm also seeing the same issue with my pull requests. I believe @lonnieezell is aware of the issue.

[baselbj]

I think the issue is in return require_once $file;

As require_once always returns Boolean and we need to return an array ... to test I changed it to require

lets see if this fixes the issue

[baselbj]

it is fixed :)

[baselbj]

I don't know if this will create another issue where the language array will be added many times

[kenjis]

As require_once always returns Boolean and we need to return an array ... to test I changed it to require

No, but almost.
I've not confirmed, but I think there is a test requires the same file more than one time.

$ cat a.php 
<?php
return [];

$ cat test.php 
<?php

var_dump(require_once './a.php'); // []
var_dump(require_once './a.php'); // true

[lonnieezell]

Hey guys, I am aware of the issue but haven't have been away due to very tight deadlines on two different projects at work. Luckily, we just wrapped these up yesterday so I'll be getting back into the swing of things and catching up on issues/PRs.

The strange thing with this one is that nothing has changed in the code at the time these tests started failing in Travis, and it all passes locally for me on a Mac, though I understand from a user in Slack that this can be replicated on a Homebrew instance. He came up with the same fix you guys did, changing that to a require, but I haven't had a chance to dig in and see what might be causing that or if it will affect anything.

Will be working on that this evening though, since it's a priority for me so that I can ensure these PR's I have to go through actually pass. :)

[baselbj]

Hi @lonnieezell I don't think this will make an issue except the array will be added multiple times.

I can't think of a good solution to that but maybe we can create our own require_once by adding each opened language file to an array and check the array to see if the file is there before adding it.

This might be a solution or to keep the require and accept the idea of having the same array uploaded multiple times

[baselbj]

in php documentation the include_once :
if the code from a file has already been included, it will not be included again, and include_once returns TRUE. As the name suggests, the file will be included just once.

So include_once return true only when the file is already included. but I reviewed the Language.php file and as I can see it already are using the same solution that I suggest before even though the issue of require_once occur.

I think the phpunit is loading the language files not using Language.php file and this causes the issue as the Language.php file didn't know that the file is loaded and try to include it.

Anyway the solution is as suggested to change requiter_once to require and the load function in Language.php will mange to not load the language array more than one time.

[lonnieezell]

@baselbj Yup. Have already committed that change. And we do actually already store the files contents in an array for our own "require once" as you say. So we should be good to go here. Closing this one.