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

Sync tests for practice exercise protein translation #2617

Merged

Conversation

manumafe98
Copy link
Contributor

@manumafe98 manumafe98 commented Jan 1, 2024

pull request

This issue addresses: #2388

The goal is to sync the tests of the protein translation practice exercise

Reviewer Resources:

Track Policies

@manumafe98 manumafe98 changed the title Sync tests for practice exercise protein translation Sync tests for practice exercise protein translation Jan 1, 2024
@manumafe98
Copy link
Contributor Author

I'm going to update the reference resolution on this one as well

Copy link
Contributor

@sanderploegsma sanderploegsma left a comment

Choose a reason for hiding this comment

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

Changes look good! I left one comment regarding the order of the tests compared to the canonical data, could you have a look at that?

Comment on lines 156 to 160
@Ignore("Remove to run test")
@Test
public void testEmptyRnaSequenceResultInNoproteins() {
assertThat(proteinTranslator.translate("")).isEmpty();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: The order of the tests in the canonical data may be ordered like that on purpose, for instance to cover basic test cases before more elaborate ones. This should aid students in their implementation.

To that end, we try to keep the order of the tests in the Java track in the same way as they are ordered in the canonical data. That would mean that this test for an empty sequence becomes the first test. Note that this also means that it shouldn't have an @Ignore tag and that the existing testTranslationOfRnaToProteinList method should be @Ignored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh okey good to know! I updated the order to match the canonical data!

@sanderploegsma sanderploegsma merged commit 816a934 into exercism:main Jan 2, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants