[1/2] support/testing: remove hardcoded sleep from python-django test

Instead of waiting for a hardcoded time of 30s we check periodically every
second if the server is already up. If it isn't up after the full timeout
(which is the same as before) expired the test fails.

We need to redirect all output of the background started task to
/dev/null now as it otherwise confuses the emulator.run() exit code
parsing logic (as it gets out of order messages from the emulator).

Signed-off-by: Marcus Hoffmann <buildroot@bubu1.eu>
---
 .../tests/package/test_python_django.py        | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

On 16.02.24 14:18, Marcus Hoffmann via buildroot wrote:
> Instead of waiting for a hardcoded time of 30s we check periodically every
> second if the server is already up. If it isn't up after the full timeout
> (which is the same as before) expired the test fails.
> 
> We need to redirect all output of the background started task to
> /dev/null now as it otherwise confuses the emulator.run() exit code
> parsing logic (as it gets out of order messages from the emulator).
> 
> Signed-off-by: Marcus Hoffmann <buildroot@bubu1.eu>
> ---
>   .../tests/package/test_python_django.py        | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/support/testing/tests/package/test_python_django.py b/support/testing/tests/package/test_python_django.py
> index e1ca50f6d8..0973467a2a 100644
> --- a/support/testing/tests/package/test_python_django.py
> +++ b/support/testing/tests/package/test_python_django.py
> @@ -1,3 +1,5 @@
> +import time
> +
>   from tests.package.test_python import TestPythonPackageBase
>   
>   
> @@ -16,13 +18,17 @@ class TestPythonDjango(TestPythonPackageBase):
>           self.assertIn("Operations to perform:", output[0])
>           self.assertEqual(exit_code, 0)
>   
> -        cmd = "cd /opt/testsite && " + self.interpreter + " ./manage.py runserver 0.0.0.0:1234 & "
> -        # give some time to setup the server
> -        cmd += "sleep {}".format(str(30 * self.emulator.timeout_multiplier))
> +        cmd = "cd /opt/testsite && " + self.interpreter + " ./manage.py runserver 0.0.0.0:1234 > /dev/null 2>&1 & "
>           self.assertRunOk(cmd, timeout=timeout)
> -
> -        cmd = "netstat -ltn 2>/dev/null | grep 0.0.0.0:1234"
> -        self.assertRunOk(cmd)
> +        # give some time to setup the server
> +        for attempt in range(30 * self.emulator.timeout_multiplier):
> +            time.sleep(1)
> +            cmd = "netstat -ltn 2>/dev/null | grep 0.0.0.0:1234"
> +            _, exit_code = self.emulator.run(cmd)
> +            if exit_code == 0:
> +                break
> +        else:
> +            self.assertTrue(False, "Timeout while waiting for django server")
>   
>   
>   class TestPythonPy3Django(TestPythonDjango):

The django test (and the whitenoise test introduced in the next patch) 
actually both currently fail because of a problem with django 5.0 and 
.pyc only installations. [1]

This now has "release blocker" priority at django and a proposed patch 
(that I tested locally but hasn't been officially submitted to django 
yet) so hopefully this will be fixed with the next django point release 
in a couple of weeks.

[1] https://code.djangoproject.com/ticket/35187

Marcus

On 16/02/2024 14:23, Marcus Hoffmann via buildroot wrote:
> On 16.02.24 14:18, Marcus Hoffmann via buildroot wrote:
>> Instead of waiting for a hardcoded time of 30s we check periodically every
>> second if the server is already up. If it isn't up after the full timeout
>> (which is the same as before) expired the test fails.
>>
>> We need to redirect all output of the background started task to
>> /dev/null now as it otherwise confuses the emulator.run() exit code
>> parsing logic (as it gets out of order messages from the emulator).
>>
>> Signed-off-by: Marcus Hoffmann <buildroot@bubu1.eu>
>> ---
>>   .../tests/package/test_python_django.py        | 18 ++++++++++++------
>>   1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/support/testing/tests/package/test_python_django.py 
>> b/support/testing/tests/package/test_python_django.py
>> index e1ca50f6d8..0973467a2a 100644
>> --- a/support/testing/tests/package/test_python_django.py
>> +++ b/support/testing/tests/package/test_python_django.py
>> @@ -1,3 +1,5 @@
>> +import time
>> +
>>   from tests.package.test_python import TestPythonPackageBase
>> @@ -16,13 +18,17 @@ class TestPythonDjango(TestPythonPackageBase):
>>           self.assertIn("Operations to perform:", output[0])
>>           self.assertEqual(exit_code, 0)
>> -        cmd = "cd /opt/testsite && " + self.interpreter + " ./manage.py 
>> runserver 0.0.0.0:1234 & "
>> -        # give some time to setup the server
>> -        cmd += "sleep {}".format(str(30 * self.emulator.timeout_multiplier))
>> +        cmd = "cd /opt/testsite && " + self.interpreter + " ./manage.py 
>> runserver 0.0.0.0:1234 > /dev/null 2>&1 & "
>>           self.assertRunOk(cmd, timeout=timeout)
>> -
>> -        cmd = "netstat -ltn 2>/dev/null | grep 0.0.0.0:1234"
>> -        self.assertRunOk(cmd)
>> +        # give some time to setup the server
>> +        for attempt in range(30 * self.emulator.timeout_multiplier):
>> +            time.sleep(1)
>> +            cmd = "netstat -ltn 2>/dev/null | grep 0.0.0.0:1234"
>> +            _, exit_code = self.emulator.run(cmd)
>> +            if exit_code == 0:
>> +                break
>> +        else:
>> +            self.assertTrue(False, "Timeout while waiting for django server")
>>   class TestPythonPy3Django(TestPythonDjango):
> 
> The django test (and the whitenoise test introduced in the next patch) actually 
> both currently fail because of a problem with django 5.0 and .pyc only 
> installations. [1]
> 
> This now has "release blocker" priority at django and a proposed patch (that I 
> tested locally but hasn't been officially submitted to django yet) so hopefully 
> this will be fixed with the next django point release in a couple of weeks.

  Ugh, and we're cutting 2024.02-rc1 right about now... I guess you can submit 
that patch directly to Buildroot then? Or is the point release going to be a 
minor one that we can still apply to the master branch in the rc period?

  Regards,
  Arnout

> 
> [1] https://code.djangoproject.com/ticket/35187
> 
> Marcus
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

Marcus, All,

On 2024-02-16 14:18 +0100, Marcus Hoffmann via buildroot spake thusly:
> Instead of waiting for a hardcoded time of 30s we check periodically every
> second if the server is already up. If it isn't up after the full timeout
> (which is the same as before) expired the test fails.
> 
> We need to redirect all output of the background started task to
> /dev/null now as it otherwise confuses the emulator.run() exit code
> parsing logic (as it gets out of order messages from the emulator).
> 
> Signed-off-by: Marcus Hoffmann <buildroot@bubu1.eu>
> ---
>  .../tests/package/test_python_django.py        | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/support/testing/tests/package/test_python_django.py b/support/testing/tests/package/test_python_django.py
> index e1ca50f6d8..0973467a2a 100644
> --- a/support/testing/tests/package/test_python_django.py
> +++ b/support/testing/tests/package/test_python_django.py
> @@ -1,3 +1,5 @@
> +import time
> +
>  from tests.package.test_python import TestPythonPackageBase
>  
>  
> @@ -16,13 +18,17 @@ class TestPythonDjango(TestPythonPackageBase):
>          self.assertIn("Operations to perform:", output[0])
>          self.assertEqual(exit_code, 0)
>  
> -        cmd = "cd /opt/testsite && " + self.interpreter + " ./manage.py runserver 0.0.0.0:1234 & "
> -        # give some time to setup the server
> -        cmd += "sleep {}".format(str(30 * self.emulator.timeout_multiplier))
> +        cmd = "cd /opt/testsite && " + self.interpreter + " ./manage.py runserver 0.0.0.0:1234 > /dev/null 2>&1 & "
>          self.assertRunOk(cmd, timeout=timeout)
> -
> -        cmd = "netstat -ltn 2>/dev/null | grep 0.0.0.0:1234"
> -        self.assertRunOk(cmd)
> +        # give some time to setup the server
> +        for attempt in range(30 * self.emulator.timeout_multiplier):
> +            time.sleep(1)
> +            cmd = "netstat -ltn 2>/dev/null | grep 0.0.0.0:1234"
> +            _, exit_code = self.emulator.run(cmd)
> +            if exit_code == 0:
> +                break
> +        else:
> +            self.assertTrue(False, "Timeout while waiting for django server")

I was not very happy that we test success against a constant that we
know is false; this does not look great..

Instead, I've slightly simplified the test: I dropped the else clause of
the for-loop, and added an asserEqual that the exit_code is indeed 0
after the loop.

Applied to master, thanks.

Regards,
Yann E. MORIN.

>  
>  class TestPythonPy3Django(TestPythonDjango):
> -- 
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot