Skip to content

Python: fix(tests): split pytest.raises blocks to eliminate dead code#5171

Open
Bahtya wants to merge 1 commit intomicrosoft:mainfrom
Bahtya:fix/pytest-raises-dead-code
Open

Python: fix(tests): split pytest.raises blocks to eliminate dead code#5171
Bahtya wants to merge 1 commit intomicrosoft:mainfrom
Bahtya:fix/pytest-raises-dead-code

Conversation

@Bahtya
Copy link
Copy Markdown

@Bahtya Bahtya commented Apr 8, 2026

Problem

Multiple with pytest.raises(...) blocks in test_types.py contain more than one statement. Since pytest.raises exits after the first exception, all subsequent statements are dead code that never execute, creating a false sense of test coverage.

Fix

Split each multi-statement pytest.raises block into separate blocks so all error cases are actually tested.

Also removed the dead detect_media_type_from_base64(data_str="") call from the base64 error block, as base64.b64decode("") is valid and does not raise.

Fixes #5115

pytest.raises exits after the first exception, so multiple statements
in a single block are dead code. Split each multi-statement
pytest.raises block into separate blocks so all error cases are
actually tested.

Also remove the dead detect_media_type_from_base64(data_str="") call
which does not raise (base64.b64decode("") is valid).

Fixes microsoft#5115

Signed-off-by: bahtya <bahtyar153@qq.com>
@moonbox3 moonbox3 added the python label Apr 8, 2026
@github-actions github-actions bot changed the title fix(tests): split pytest.raises blocks to eliminate dead code Python: fix(tests): split pytest.raises blocks to eliminate dead code Apr 8, 2026
@Serjbory
Copy link
Copy Markdown

Serjbory commented Apr 9, 2026

@Bahtya, thanks a lot for your PR! I don’t think removing the test case is the right approach here.

If the test was added, it likely represents the intended behavior. In that case, the implementation should probably be updated to make the test pass. Alternatively, it would be good to get confirmation from a maintainer (for example, @moonbox3) before proceeding this way, as outlined in the contribution guidelines.

@eavanvalkenburg
Copy link
Copy Markdown
Member

@Bahtya, thanks a lot for your PR! I don’t think removing the test case is the right approach here.

If the test was added, it likely represents the intended behavior. In that case, the implementation should probably be updated to make the test pass. Alternatively, it would be good to get confirmation from a maintainer (for example, @moonbox3) before proceeding this way, as outlined in the contribution guidelines.

as far as I can see, this does not remove any tests, just ensures that each check actually runs.

@Serjbory
Copy link
Copy Markdown

@Bahtya, thanks a lot for your PR! I don’t think removing the test case is the right approach here.
If the test was added, it likely represents the intended behavior. In that case, the implementation should probably be updated to make the test pass. Alternatively, it would be good to get confirmation from a maintainer (for example, @moonbox3) before proceeding this way, as outlined in the contribution guidelines.

as far as I can see, this does not remove any tests, just ensures that each check actually runs.

It removes detect_media_type_from_base64(data_str="") case. According to the original test-suit: the empty string has to caus "Invalid base64 data provided." error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: pytest.raises wrong usage (dead code and unexpected behaviour)

4 participants