Skip to content

[3.6] bpo-34272: Restore test_capi.EmbeddingTests, unintentionally removed in 278d975c#10078

Merged
serhiy-storchaka merged 1 commit intopython:3.6from
twosigma:restore-embeddingtests
Oct 30, 2018
Merged

[3.6] bpo-34272: Restore test_capi.EmbeddingTests, unintentionally removed in 278d975c#10078
serhiy-storchaka merged 1 commit intopython:3.6from
twosigma:restore-embeddingtests

Conversation

@lordmauve
Copy link
Copy Markdown
Contributor

@lordmauve lordmauve commented Oct 24, 2018

I believe some tests were unintentionally dropped from the 3.6 branch in 278d975 as a result of a cherry-pick from 8f7bb10 by @serhiy-storchaka.

In master, EmbeddingTests has moved to Lib/test/test_embed.py, so I think the cherry-pick may have seen this as a removal and applied it to the 3.6 branch. It was sandwiched between two other classes that were removed intentionally so perhaps git saw it as one hunk?

This PR restores the missing test case to the 3.6 branch.

https://bugs.python.org/issue34272

@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting review labels Oct 24, 2018
@lordmauve lordmauve changed the title Restore test_capi.EmbeddingTests, unintentionally removed in 278d975c [3.6] Restore test_capi.EmbeddingTests, unintentionally removed in 278d975c Oct 24, 2018
@vstinner vstinner changed the title [3.6] Restore test_capi.EmbeddingTests, unintentionally removed in 278d975c [3.6] bpo-34272: Restore test_capi.EmbeddingTests, unintentionally removed in 278d975c Oct 24, 2018
@vstinner
Copy link
Copy Markdown
Member

Rather than adding back tests to test_capi.py, maybe it would be better to create Lib/test/test_embed.py to be closer to 3.7 and master branches, to ease future backports.

What do you think @serhiy-storchaka? cc @ncoghlan @ericsnowcurrently

@ncoghlan
Copy link
Copy Markdown
Contributor

I'm OK with the idea of backporting the refactoring, but I think it would be good to restore the test case in the original location first, so commit moving it is clearer.

@serhiy-storchaka
Copy link
Copy Markdown
Member

Thank you for catching this @lordmauve!

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

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants