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

Why test with exception in test_import? #241

Closed
DrDomenicoMarson opened this issue Sep 24, 2022 · 5 comments · Fixed by #240
Closed

Why test with exception in test_import? #241

DrDomenicoMarson opened this issue Sep 24, 2022 · 5 comments · Fixed by #240
Labels

Comments

@DrDomenicoMarson
Copy link
Contributor

I was checking for coverage and I saw this test

import alchemlyb 
 
def test_name(): 
    try: 
        assert alchemlyb.__name__ == 'alchemlyb' 
    except Exception as e: 
        raise e 

Why are we catching an Exception here in a test?

@xiki-tempula
Copy link
Collaborator

No idea. It seems to me that assert alchemlyb.__name__ == 'alchemlyb' should be enough. Could you use blame to check who wrote that part?

@DrDomenicoMarson
Copy link
Contributor Author

I didn't know about "blame" :-)

It seems @ianmkenney did the commit 6 years ago! Could we just remove the try?

@ianmkenney
Copy link
Collaborator

I couldn't tell you why I thought that was either necessary or useful. Feel free to change!

@DrDomenicoMarson
Copy link
Contributor Author

Great, I implemented the change in PR #240, I hope it's fine, just not to have another PR just for that!

@orbeckst
Copy link
Member

If you had done another PR, I would have just merged it.

Micro-PRs are easier to maintain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants