-
Notifications
You must be signed in to change notification settings - Fork 127
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
Task #3132: Removing AtCompositeTest.TO_ADD_MESSAGE #3187
Conversation
…AILED_ASSERT_MESSAGE_SUPPLIER in tests
@ZacParize thanks for your pull request. To start review we need all GHA jobs are green. You have only 5 qulice violations which is easy to resolve. Tag me if you need help |
@maxonfjvipon Thank you for your answer. I've seen logs, there are issues connected with stands (EOF, unable to delete directory and so on). Please, give me a clue what i have to do. |
@ZacParize these are the violations you need to fix:
|
@maxonfjvipon Looks like all checks have completed. |
@ZacParize thanks again for your contribution, but this ticket was about to replace static string with some Meaningful message which would help a programmer to understand what exactly is wrong. You can read more here about it. |
…atTest & EOcageTest
@maxonfjvipon Please, check out my last commit. Have i choose right test message format for completing the task? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ZacParize thanks, that's better but there are a few things that can be improved
TODO:
replace all appearances of AtCompositeTest.TO_ADD_MESSAGE field in eo-runtime with meaningful assert messages
and don't forget to remove public modifier from this class if no longer need.
DONE:
Moved from AtCompositeTest.TO_ADD_MESSAGE to AtCompositeTest.FAILED_ASSERT_MESSAGE_SUPPLIER in tests because of more reasonable assert message.
Didn't created own FAILED_ASSERT_MESSAGE_SUPPLIER in every test class because of boilerplate.
It doesn't make any sense.
Left public modifier for AtCompositeTest because it reuses in other tests.
PR-Codex overview
This PR updates test assertions in multiple test files to use
AtCompositeTest.FAILED_ASSERT_MESSAGE_SUPPLIER.get()
instead ofAtCompositeTest.TO_ADD_MESSAGE
.Detailed summary
AtCompositeTest.TO_ADD_MESSAGE
withAtCompositeTest.FAILED_ASSERT_MESSAGE_SUPPLIER.get()
in multiple test files for improved test assertion messages.