TheAlgorithms / Python

All Algorithms implemented in Python
https://the-algorithms.com/
MIT License
188.95k stars 44.89k forks source link

Improve our test coverage #9943

Open tianyizheng02 opened 1 year ago

tianyizheng02 commented 1 year ago

Feature description

Many of our existing algorithm files have little to no unit testing. This is problematic because this can easily let bugs slip through. We want some assurance that the code we currently have is correct and functional. We welcome all contributors to open PRs to help us add tests to our codebase.

How to find low-coverage files

Go to the Actions tab in this repository and find the most recent build workflow run. Open the logs under "Run Tests" and scroll down until you find the section on code coverage:

---------- coverage: platform linux, python 3.12.0-final-0 -----------
Name                                                                          Stmts   Miss  Cover   Missing
-----------------------------------------------------------------------------------------------------------
quantum/q_fourier_transform.py                                                   30     30     0%   14-93
scripts/validate_solutions.py                                                    54     54     0%   2-94
strings/min_cost_string_conversion.py                                            78     75     4%   20-57, 61-75, 79-129
...

The "Cover" column tells you what percentage of the lines in that file are covered by tests. We want to increase this percentage for existing files. Find a file with low coverage percentage that you wish to write tests for, add doctests for each function, and open a PR with your changes. You do not need to have a perfect coverage percentage, but all functions should have doctests.

Some files will naturally be hard to write tests for. For example, the file may be poorly written because they lack any functions. Other files might be how-tos, meaning they simply demonstrate how to use an existing library's functions rather than implementing the algorithm themselves. Ignore these kinds of files, as they will need to be rewritten eventually. Furthermore, ignore files in the web_programming and project_euler directories. Web programming files are inherently hard to test and Project Euler files have their own validation workflow, so don't worry about their test coverage.

When you open your PR, put "Contributes to #9943" in the PR description. Do not use the word "fixes", "resolves", or "closes". This issue is an ongoing one, and your PR will not single-handedly resolve this issue.

How to add doctests

A doctest is a unit test that is contained within the documentation comment (docstring) for a function. Here is an example of what doctests look like within a docstring:

def add(a: int, b: int) -> int:
    """
    Adds two non-negative numbers.
    >>> add(1, 1)
    2
    >>> add(2, 5)
    7
    >>> add(1, 0)
    1
    >>> add(-1, -1)
    Traceback (most recent last):
    ...
    ValueError: Numbers must be non-negative
    """

For every function in the file you choose, you should write doctests like the ones shown above in its docstring. If a function doesn't have a docstring, add one. Your doctests should be comprehensive but not excessive: you should write just enough tests to cover all basic cases as well as all edge cases (e.g., negative numbers, empty lists, etc).

Do not simply run a function on some example inputs and put its output as the expected output for a doctest. This assumes that the function is implemented correctly when it might not be. Verify independently that your doctests and their expected outputs are correct. Your PR will not be merged if it has failing tests. If you happen to discover a bug while writing doctests, please fix it.

Please read our contributing guidelines before you contribute.

SaiHarshaK commented 12 months ago

I'll work on the palindrome script

AnshuSharma111 commented 12 months ago

Hi. I am new and would like to contribute to this issue

SaiHarshaK commented 12 months ago

Please check:

AasheeshLikePanner commented 12 months ago

@tianyizheng02 Can you please Review this two PRs

Adding doctests in simpson_rule.py

Adding doctests in longest_sub_array.py

harjotjuneja6 commented 11 months ago

Can we close this issue because it is already merged.

tianyizheng02 commented 11 months ago

No, improving test coverage is an ongoing issue. It can't be resolved by a single PR because there are many, many files that need tests. Right now, it definitely hasn't been resolved.

AasheeshLikePanner commented 11 months ago

@tianyizheng02 Can you please review this PR

SaiHarshaK commented 11 months ago

Can we ask people to use "Ref" instead "Fixes", so that we don't have keep reopening this issue?

RaymondDashWu commented 11 months ago

How is the percentage determined? The tests I added only bumped the coverage by 1%

tianyizheng02 commented 11 months ago

How is the percentage determined? The tests I added only bumped the coverage by 1%

@RaymondDashWu The coverage percentage is calculated from the total number of lines covered by tests across all files in the repo. As of writing, the latest builds show the following:

-----------------------------------------------------------------------------------------------------------
TOTAL                                                                         31846   9986    69%

There are 9986 lines of code in total not covered by tests compared to 31846 lines of code in total, which is how the 69% comes about. The tests that you added only improved the coverage for one file out of the hundreds of files that we have, and that's why it barely changed the percentage at all.

This is why I said in my previous comment that this is an ongoing issue. No single PR will improve our coverage enough to "fix" this issue.

R055A commented 11 months ago

Hi @tianyizheng02 Have code coverage improved from 16% to 85% (or 100% when not including the main block) for the graphs/dijkstra_algorithm.py file (linked above)

RaymondDashWu commented 11 months ago

d by tests compared to 31846 lines of code in total, which is how the 69% comes about. The tests that you added only improved the coverage for one file out of the hundreds of files that we have, and that's why it barely changed the percentage at all.

Thanks for this info @tianyizheng02! I figured out that I've actually been looking at the wrong percentages for test coverage. I was looking at the loading of the test session percentages thinking that's what you pointed out in the first screenshot

image
nkstonks commented 11 months ago

Hi, I was looking at the Fibonacci function, and I found an untested function "fib_recursive_term" that was nested inside of the fib_recursive function. I added tests for it, so in terms of this issue it's all good now.

But I was just wondering whether or not I should define that function globally because I don't see the point of having it nested in the fib_recursive function.

So I want to propose to move the fib_recursive_term out from the fib_recursive function and have it as a separate function above it, would there be any issue if that happens?

SaiHarshaK commented 11 months ago

Maybe you can create a new issue for it since the scope of this issue seems to be only addition of do tests?

On Sat, Oct 21, 2023, 2:40 PM Kento @.***> wrote:

Hi, I was looking at the Fibonacci function, and I found an untested function "fib_recursive_term" that was nested inside of the fib_recursive function. I added tests for it, so in terms of this issue it's all good now.

But I was just wondering whether or not I should define that function globally because I don't see the point of having it nested in the fib_recursive function.

So I want to propose to move the fib_recursive_term out from the fib_recursive function and have it as a separate function above it, would there be any issue if that happens?

— Reply to this email directly, view it on GitHub https://github.com/TheAlgorithms/Python/issues/9943#issuecomment-1773726996 or unsubscribe https://github.com/notifications/unsubscribe-auth/AHJ6I7YRX4SZLGETK25ZLCDYAOGPZBFKMF2HI4TJMJ2XIZLTS2BKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLAVFOZQWY5LFVE2DAOJRGUYTAOJTURXGC3LFVFUGC427NRQWEZLMQKSXMYLMOVS2SNBQHEYTKMJQHE2KI3TBNVS2S2DBONPWYYLCMVWIFJLWMFWHKZNJHEYDENJWGQ4TAN5ENZQW2ZNJNBQXGX3MMFRGK3FMON2WE2TFMN2F65DZOBS2YSLTON2WKQ3PNVWWK3TUUZ2G64DJMNZZLAVEOR4XAZNKOJSXA33TNF2G64TZUV3GC3DVMWUDMMZUG43DGMZXQKSHI6LQMWSWS43TOVS2K5TBNR2WLKRRHEZTAOBWHEYTSMUCUR2HS4DFUVWGCYTFNSSXMYLMOVS2SNBQHEYTKMJQHEZYFJDUPFYGLJLMMFRGK3FFOZQWY5LFVE2DAOJRGUYTAOJUQKSHI6LQMWSWYYLCMVWKK5TBNR2WLKJZGAZDKNRUHEYDPJ3UOJUWOZ3FOKTGG4TFMF2GK . You are receiving this email because you commented on the thread.

Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub .

PreciousJac0b commented 11 months ago

@tianyizheng02 Please help review my pull request so I can write more tests for this repo. https://github.com/TheAlgorithms/Python/pull/10751

nkstonks commented 11 months ago

@SaiHarshaK alright, ill make a new issue for the nested functions thing, meanwhile I'll make a pull request where I've implemented doctests to several functions once I double check everything

imSanko commented 11 months ago

@cclauss Now we need to work to FIX the failing TESTS for improving, while its of #10822

vanlucard commented 7 months ago

It's like Fourier Sistem i thought

Saty70 commented 5 months ago

hey I want add some tests to the algorithm. can someone guide the prerequisite that I need to know to add test cases will it be implemented by Pytest or something else

cclauss commented 5 months ago

https://github.com/TheAlgorithms/Python/blob/1868c0b6375188a9034478a2711e40c343d00c2e/.github/workflows/build.yml#L27-L32

HersiYussuf commented 4 months ago

Hey! I am new to opensource I would Love to contribute, any tips and ideas on how can I safely and successfully contribute

Argyrhspet1 commented 2 months ago

Hello everyone,I am getting started with open source and I would like to work on this issue .Can someone guide me on what to do?Thank you

Amul-Sharma-404 commented 2 months ago

for the coverage report section:

---------- coverage: platform linux, python 3.12.0-final-0 -----------
Name                                                                          Stmts   Miss  Cover   Missing
-----------------------------------------------------------------------------------------------------------
quantum/q_fourier_transform.py                                                   30     30     0%   14-93
scripts/validate_solutions.py                                                    54     54     0%   2-94
strings/min_cost_string_conversion.py                                            78     75     4%   20-57, 61-75, 79-129
...

# code for the how to add doctests
def function_name(param1: type, param2: type) -> return_type:

      # Description of what the function does.

      # function_name(input1, input2)
        # expected_output

    # Add more tests to cover different cases
     #   function_name(other_input1, other_input2)
    #   expected_output
Akash-Tiwari-890 commented 1 week ago

def add(a, b): """ Adds two numbers and returns the result.

>>> add(2, 3)
5
>>> add(-1, 1)
0
"""
return a + b

we can use this also

cclauss commented 1 week ago

@Akash-Tiwari-890 We do in 800+ files...

DhanushA1307 commented 4 days ago

Hi. I am new and would like to contribute to this issue @tianyizheng02

HersiYussuf commented 4 days ago

Okay have at it.

On Wed, 2 Oct 2024, 11:28 Dhanush A, @.***> wrote:

Hi. I am new and would like to contribute to this issue @tianyizheng02 https://github.com/tianyizheng02

— Reply to this email directly, view it on GitHub https://github.com/TheAlgorithms/Python/issues/9943#issuecomment-2387908828, or unsubscribe https://github.com/notifications/unsubscribe-auth/A3TF57TZQ3VRJCIFMIR4LZTZZOVC5AVCNFSM6AAAAABIVUILECVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBXHEYDQOBSHA . You are receiving this because you commented.Message ID: @.***>

mustalidhanerawala commented 4 days ago

Hello, I am new over here, Is this issue still open?

tianyizheng02 commented 4 days ago

Yes, this issue is still open.

@DhanushA1307 @mustalidhanerawala We don't assign issues in this repo. If you want to work on it, just open a PR with your changes. Our contributing guidelines already say this, so read those first if you haven't already.

vanton commented 15 hours ago

Any new progress?