8000 Remove incorrect `@expectedFailure`s from `test_cmd_line` by sobolevn · Pull Request #5201 · RustPython/RustPython · GitHub
[go: up one dir, main page]

Skip to content

Remove incorrect @expectedFailures from test_cmd_line #5201

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

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

sobolevn
Copy link
Contributor

After you suggestion in python/cpython#116504 (comment) I went to take a look at test_cmd_line in RustPython (it was so long ago I contributed to this amazing project, so may thing had changed!), and I've noticed this.

This is a problem, here' the simplest demo:

import unittest

class TestMe(unittest.TestCase):
    @unittest.expectedFailure
    def test_me(self):
        def run():
            raise ValueError

        with self.subTest(run=run):
            run()

if __name__ == '__main__':
    unittest.main()

This works as expected:

» ./python.exe ex.py
x
----------------------------------------------------------------------
Ran 1 test in 0.001s

OK (expected failures=1)

This does not:

import unittest

class TestMe(unittest.TestCase):
    def test_me(self):
        @unittest.expectedFailure
        def run():
            raise ValueError

        with self.subTest(run=run):
            run()

if __name__ == '__main__':
    unittest.main()

Produces:

» ./python.exe ex.py
E
=================
8000
=====================================================
ERROR: test_me (__main__.TestMe.test_me) (run=<function TestMe.test_me.<locals>.run at 0x1057a2150>)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython2/ex.py", line 10, in test_me
    run()
    ~~~^^
  File "/Users/sobolev/Desktop/cpython2/ex.py", line 7, in run
    raise ValueError
ValueError

----------------------------------------------------------------------
Ran 1 test in 0.001s

FAILED (errors=1)

So, I propose to remove these decorators, let's only keep TODO comments to indicate separate failures.

After you suggestion in python/cpython#116504 (comment) I went to take a look at `test_cmd_line` in RustPython (it was so long ago I contributed to this amazing project, so may thing had changed!), and I've noticed this.

This is a problem, here' the simplest demo:

```python
import unittest

class TestMe(unittest.TestCase):
    @unittest.expectedFailure
    def test_me(self):
        def run():
            raise ValueError

        with self.subTest(run=run):
            run()

if __name__ == '__main__':
    unittest.main()
```

This works as expected:

```
» ./python.exe ex.py
x
----------------------------------------------------------------------
Ran 1 test in 0.001s

OK (expected failures=1)
```

This does not:

```python
import unittest

class TestMe(unittest.TestCase):
    def test_me(self):
        @unittest.expectedFailure
        def run():
            raise ValueError

        with self.subTest(run=run):
            run()

if __name__ == '__main__':
    unittest.main()
```

Produces:

```
» ./python.exe ex.py
E
======================================================================
ERROR: test_me (__main__.TestMe.test_me) (run=<function TestMe.test_me.<locals>.run at 0x1057a2150>)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython2/ex.py", line 10, in test_me
    run()
    ~~~^^
  File "/Users/sobolev/Desktop/cpython2/ex.py", line 7, in run
    raise ValueError
ValueError

----------------------------------------------------------------------
Ran 1 test in 0.001s

FAILED (errors=1)
```

So, I propose to remove these decorators, let's only keep `TODO` comments to indicate separate failures.
Copy link
Member
@youknowone youknowone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, and nice to see you again here too 😆

@youknowone youknowone merged commit 426e582 into RustPython:main Mar 15, 2024
kingiler pushed a commit to kingiler/RustPython that referenced this pull request Mar 15, 2024
…#5201)

After you suggestion in python/cpython#116504 (comment) I went to take a look at `test_cmd_line` in RustPython (it was so long ago I contributed to this amazing project, so may thing had changed!), and I've noticed this.

This is a problem, here' the simplest demo:

```python
import unittest

class TestMe(unittest.TestCase):
    @unittest.expectedFailure
    def test_me(self):
        def run():
            raise ValueError

        with self.subTest(run=run):
            run()

if __name__ == '__main__':
    unittest.main()
```

This works as expected:

```
» ./python.exe ex.py
x
----------------------------------------------------------------------
Ran 1 test in 0.001s

OK (expected failures=1)
```

This does not:

```python
import unittest

class TestMe(unittest.TestCase):
    def test_me(self):
        @unittest.expectedFailure
        def run():
            raise ValueError

        with self.subTest(run=run):
            run()

if __name__ == '__main__':
    unittest.main()
```

Produces:

```
» ./python.exe ex.py
E
======================================================================
ERROR: test_me (__main__.TestMe.test_me) (run=<function TestMe.test_me.<locals>.run at 0x1057a2150>)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython2/ex.py", line 10, in test_me
    run()
    ~~~^^
  File "/Users/sobolev/Desktop/cpython2/ex.py", line 7, in run
    raise ValueError
ValueError

----------------------------------------------------------------------
Ran 1 test in 0.001s

FAILED (errors=1)
```

So, I propose to remove these decorators, let's only keep `TODO` comments to indicate separate failures.
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

Successfully merging this pull request may close these issues.

2 participants
0