Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_get_normally_distributed_number is flaky #7081

Closed
devos50 opened this issue Oct 18, 2022 · 3 comments
Closed

test_get_normally_distributed_number is flaky #7081

devos50 opened this issue Oct 18, 2022 · 3 comments
Assignees
Milestone

Comments

@devos50
Copy link
Contributor

devos50 commented Oct 18, 2022

Since the test_get_normally_distributed_number test depends on randomness, it can be flaky and fail when 'unlucky' numbers are drawn that do not pass the Shapiro test. I saw this test failing in one of my runs:

2022-10-18T07:43:01.6956570Z =================================== FAILURES ===================================
2022-10-18T07:43:01.6957363Z _____________________ test_get_normally_distributed_number _____________________
2022-10-18T07:43:01.6958298Z 
2022-10-18T07:43:01.6958890Z     def test_get_normally_distributed_number():
2022-10-18T07:43:01.6959807Z         """
2022-10-18T07:43:01.6960279Z         To test if the random number is from normal distribution, we do Shapiro test and check that
2022-10-18T07:43:01.6961011Z         p-value is higher than 0.05.
2022-10-18T07:43:01.6961267Z         """
2022-10-18T07:43:01.6961633Z         random_numbers = [get_normally_distributed_number_with_zero_mean() for _ in range(100)]
2022-10-18T07:43:01.6962039Z         shapiro_test = shapiro(random_numbers)
2022-10-18T07:43:01.6962386Z >       assert shapiro_test.pvalue > 0.05
2022-10-18T07:43:01.6962673Z E       assert 0.005304783117026091 > 0.05
2022-10-18T07:43:01.6963047Z E        +  where 0.005304783117026091 = ShapiroResult(statistic=0.9616765379905701, pvalue=0.005304783117026091).pvalue
2022-10-18T07:43:01.6963294Z 
2022-10-18T07:43:01.6963496Z src/tribler/core/utilities/tests/test_utilities.py:270: AssertionError
2022-10-18T07:43:01.6964125Z =============================== warnings summary ===============================

I also managed to make this test fail locally, by running this particular test hundreds of times:

seq 1000 | xargs -Iz pytest tribler/core/utilities/tests/test_utilities.py::test_get_normally_distributed_number

================================================================================================================================== FAILURES ==================================================================================================================================
____________________________________________________________________________________________________________________ test_get_normally_distributed_number ____________________________________________________________________________________________________________________

    def test_get_normally_distributed_number():
        """
        To test if the random number is from normal distribution, we do Shapiro test and check that
        p-value is higher than 0.05.
        """
        random_numbers = [get_normally_distributed_number_with_zero_mean() for _ in range(100)]
        shapiro_test = shapiro(random_numbers)
>       assert shapiro_test.pvalue > 0.05
E       assert 0.03620334342122078 > 0.05
E        +  where 0.03620334342122078 = ShapiroResult(statistic=0.972777783870697, pvalue=0.03620334342122078).pvalue

tribler/core/utilities/tests/test_utilities.py:270: AssertionError

I'm not in favour of having such a test. I'm also questioning its value since we're essentially testing whether the random.normalvariate method gives correct numbers.

@devos50 devos50 added this to the 7.13.0 milestone Oct 18, 2022
@drew2a
Copy link
Contributor

drew2a commented Oct 19, 2022

@xoriole seems to be a code owner of these tests.

I confirm that test_get_normally_distributed_number probably is the most unstable test at the moment.

@xoriole xoriole self-assigned this Oct 19, 2022
@drew2a
Copy link
Contributor

drew2a commented Oct 21, 2022

@xoriole
Copy link
Contributor

xoriole commented Nov 7, 2022

#7103 resolves this

@xoriole xoriole closed this as completed Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants