Add translate_from feature to translator_cog and its tests

[Eyad-Jawad]

• Add the feature on issue #127, which is to add translate from to translator_cog
• Add tests for translator_cog and the helper function language_autocomplete
• Add conftest.py to be used for all test files

[chrisdedman]

LGTM. Other than the test method that does not follow our convention (please, make changes), I am good with it. Please, @PenguinBoi12 have a second review.

[PenguinBoi12]

In general it looks good to me but has @chrisdedman pointed out, we prefer when test follow this convention: test_<function/method/action>__expect_<result> and test_<function/method/action>__<modifier>__expect_<result>

For example:

• test_translator becomes test_translator__expect_embed_with_translation
• test_language_autocomplete_ara_input becomes test_language_autocomplete__with_partial_input__expect_matching_languages

You also need to test both the happy paths (test works as intended) and unhappy paths. For example, you should test what happens when the translators receive a bad input (like a language that doesn't exists). Many of your test do not test those correctly.

Otherwise, good job! It's a good first open-source contribution.

[PenguinBoi12]

This test isn't actually mocking Translator, so it's calling the real googletrans API to trigger the ValueError. That makes the test flaky and dependent on network access, which we want to avoid in unit tests. You patch Translator here like you did in the happy path test, and raise the error from the mock directly.

@pytest.mark.asyncio
@patch("bot.extensions.translator_cog.Translator")
async def test_translator__with_invalid_input__expect_error(
    mock_translator, translator_cog
):
    ...

    instance = mock_translator.return_value
    instance.translate.side_effect = ValueError("invalid language code")

    ....

[PenguinBoi12]

You often mock defer but you don't test that its called:

ctx.defer.assert_awaited_once()

[PenguinBoi12]

You added defaults for translate_from ("auto") and translate_into ("english") earlier in review, but there's no test covering that path. Add one that omits both kwargs and confirms they get passed through to Translator().translate(), one test that passes each and one that passes both.

[PenguinBoi12]

This only checks len(empty) == 25, not which 25 came back. If the slicing logic ever grabs the wrong subset, this test still passes. We need to tests the actual content too.

[PenguinBoi12]

The expected Embed here doesn't match what the cog actually produces. The cog does translate_into.capitalize() for the field name and translated_text.text.capitalize() for the value, but this test builds the expected result with the raw, uncapitalized strings. Run this test locally and confirm whether it's actually passing right now. Update the expected embed to match the capitalized output.

[PenguinBoi12]

You should check the message too. It should be "Please enter a valid language code."

[PenguinBoi12]

It might worth to make a fixture for the ctx:

@pytest.fixture
def mock_ctx():
    """Create a mock command context."""
    ctx = MagicMock()
    ctx.defer = AsyncMock()
    ctx.send = AsyncMock()
    return ctx

[PenguinBoi12]

There's no case sensitivity test of language_autocomplete

@pytest.mark.asyncio
async def test_language_autocomplete__with_uppercase_input__expect_case_insensitive_match():
    result = await language_autocomplete(None, "ARA")
    assert result == [
        Choice(name="Arabic", value="arabic"),
        Choice(name="Gujarati", value="gujarati"),
        Choice(name="Marathi", value="marathi"),
    ]

[PenguinBoi12]

nit: you could've used parametrize

[PenguinBoi12]

LGTM 🦭

Addressed