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

meetup: incorrect test #476

Closed
kytrinyx opened this issue Jun 4, 2017 · 1 comment
Closed

meetup: incorrect test #476

kytrinyx opened this issue Jun 4, 2017 · 1 comment

Comments

@kytrinyx
Copy link
Member

kytrinyx commented Jun 4, 2017

@Ef-Eff reported in exercism/exercism#3552:

The last test for the Python challenge Meetup is written incorrectly, it is simply missing the parentheses for the function after the exception.
This:
def test_nonexistent_fifth_monday_of_february_2015(self): self.assertRaises(MeetupDayException, meetup_day, 2015, 2, 'Monday', '5th')
Should be this:
def test_nonexistent_fifth_monday_of_february_2015(self): self.assertRaises(MeetupDayException, meetup_day(2015, 2, 'Monday', '5th'))

After making this change, the test worked as intended.


@catb0t noted:

this is the relevant line and file, and 0d6d90f introduced it.


Here's our documentation about exercise test suites on Exercism in general:
https://github.com/exercism/docs/blob/master/contributing-to-language-tracks/exercise-test-suites.md

Check out the README and CONTRIBUTING guide in this repository to check for considerations specific to Python.

@behrtam
Copy link
Contributor

behrtam commented Jun 4, 2017

It is written correctly as it is right now.

If you take a look at the python docs (see this) you will see that assertRaises(exception, callable, *args, **kwds) takes a callable and than the arguments for that. This way you don't actually call the function your self.

While it's correct, I still think we could improve it. There is another way to test exceptions added with Python 2.7. If you use a context manger instead you can make the call your self which makes it more understandable/explicit, but they do (test) exactly the same thing.

def test_it(self):
    self.assertRaises(SomeException, do_something, 42, True)

def test_it_with_context(self):
    with self.assertRaises(SomeException):
        do_something(42, True)

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

No branches or pull requests

2 participants