Skip to content
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

Add a test suite for TranslationServer #79317

Closed

Conversation

azuloo
Copy link
Contributor

@azuloo azuloo commented Jul 11, 2023

A test suit that covers TranslationServer, see #43440 for more details.

@azuloo azuloo requested a review from a team as a code owner July 11, 2023 07:48
@AThousandShips AThousandShips added this to the 4.x milestone Jul 11, 2023
@azuloo azuloo force-pushed the translation-server-test-suit branch from 52cc7f7 to e7c3ffe Compare July 11, 2023 10:46
@akien-mga akien-mga changed the title Add a test suit for TranslationServer Add a test suite for TranslationServer Jul 11, 2023
@azuloo
Copy link
Contributor Author

azuloo commented Jul 11, 2023

I've fixed formatting issues and removed unneeded includes. Also squished 2 commits into one. There's also a typo in my commit message ('suit' -> 'suite'), should I change it?

int l_count_before = ts->get_loaded_locales().size();
ts->add_translation(t);
int l_count_after = ts->get_loaded_locales().size();
// Newly created Translation object should be added to the list, so the counter should increase, too
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Newly created Translation object should be added to the list, so the counter should increase, too
// Newly created Translation object should be added to the list, so the counter should increase, too.

Comments should start with a capital and end with a period, same elsewhere


CHECK(res == loc);

// No such variant in variant_map; should return everyting except the variant
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// No such variant in variant_map; should return everyting except the variant
// No such variant in variant_map; should return everything except the variant

@AThousandShips
Copy link
Member

Go ahead and update the commit message spelling when fixing the issues

@azuloo azuloo closed this Jul 11, 2023
@azuloo azuloo force-pushed the translation-server-test-suit branch from e7c3ffe to ef155c1 Compare July 11, 2023 10:57
@AThousandShips
Copy link
Member

You accidentally reset your branch to master, automatically closing the PR, you need to open a new one

@AThousandShips AThousandShips removed this from the 4.x milestone Jul 11, 2023
@AThousandShips AThousandShips removed the request for review from a team July 11, 2023 11:01
@Calinou
Copy link
Member

Calinou commented Jul 13, 2023

Superseded by #79331.

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

Successfully merging this pull request may close these issues.

3 participants