pytest-dev / unittest2pytest

helps rewriting Python `unittest` test-cases into `pytest` test-cases
GNU General Public License v3.0
128 stars 27 forks source link

Missing trailing : on emitted with statement #18

Open jayvdb opened 7 years ago

jayvdb commented 7 years ago

The tool chokes on https://github.com/coala/coala/blob/8a25983/tests/parsing/CliParsingTest.py#L58

Somehow it writes with statements without a trailing :.

         sections = parse_cli(arg_list=['--relpath'])
-        with self.assertRaisesRegex(SystemExit, '2') as cm:
+        with pytest.raises(SystemExit) as excinfo
+        assert re.search(pattern, excinfo.value) as cm:
             check_conflicts(sections)
-            self.assertEqual(cm.exception.code, 2)
+            assert cm.exception.code == 2

         sections = parse_cli(arg_list=['--output', 'iraiseValueError'])
-        with self.assertRaisesRegex(SystemExit, '2') as cm:
+        with pytest.raises(SystemExit) as excinfo
+        assert re.search(pattern, excinfo.value) as cm:
             check_conflicts(sections)
-            self.assertEqual(cm.exception.code, 2)
+            assert cm.exception.code == 2

The result is naturally a syntax error: https://travis-ci.org/jayvdb/coala/jobs/246949584#L9068

jayvdb commented 7 years ago

I suspect it is failing on with .. as ..:

htgoebel commented 7 years ago

Can you please provide a minimal example and use context-diff. I can't spot what is changed here. Thanks.

htgoebel commented 7 years ago

The problem is that within

with self.assertRaisesRegex(SystemExit, '2') as cm:

the self.assertRaisesRegex(…) gets expanded to

pytest.raises(SystemExit) as excinfo:
        assert re.search(pattern, excinfo.value)

Easy As a quick solution we could do this when converting the code

Help wanted: In the long-run:

nicoddemus commented 7 years ago

FWIW since 2.10 pytest.raises as context-manager supports a match keyword which matches with re.search, so:

with self.assertRaisesRegex(SystemExit, '2') as cm:

Can be rewritten directly as:

with pytest.raises(SystemExit, match='2') as cm:
htgoebel commented 7 years ago

Using the match keyword could be an elegant solution, but it would impose that any test-suite converted with unittest2pytest requires an up-to-date version of pytest.

I'm fine with this, since any project converting now ought not have "pinned" to a specific version of pytest. But we should be aware of this, decide about it and document.

For the records: Even if the docs say this was added in 2.10, the changelog says it was added in 3.1.0 (2017-05-22)

nicoddemus commented 7 years ago

For the records: Even if the docs say this was added in 2.10, the changelog says it was added in 3.1.0 (2017-05-22)

Oops thanks for catching that, I will fix it in the docs. 👍

nicoddemus commented 7 years ago

xref: pytest-dev/pytest#2697