Skip to content

Conversation

@Katerix
Copy link
Contributor

@Katerix Katerix commented Feb 7, 2023

dev

JIRA

Code reviewers

  • @github_username

Second Level Review

  • @github_username

Summary of issue

ToDo

Summary of change

ToDo

Testing approach

ToDo

CHECK LIST

  • СI passed
  • Сode coverage >=95%
  • PR is reviewed manually again (to make sure you have 100% ready code)
  • All reviewers agreed to merge the PR
  • I've checked new feature as logged in and logged out user if needed
  • PR meets all conventions

@Katerix Katerix changed the base branch from master to UnitTests February 7, 2023 14:12
@Katerix Katerix changed the title Additional context unittests additional-content-unittests Feb 7, 2023
An answer for the previous change request. Changed naming and moved setups to private methods.
@Katerix Katerix requested a review from EyR1oN February 9, 2023 09:25
Copy link
Contributor

@Tysyatsky Tysyatsky left a comment

Choose a reason for hiding this comment

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

On my humble opinion, it isn't critical to change, but it will make your tests cleaner and more straight forward.

Comment on lines +35 to +38
Id = 2,
Status = SubtitleStatus.Illustrator,
StreetcodeId = 1
}
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, use a constant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even in GetAll? It will be used once, is it really mandatory?

Followed Natalia's advices
@Katerix Katerix merged commit 4a4b71a into UnitTests Feb 9, 2023
@Katerix Katerix deleted the additional-context-unittests branch February 19, 2023 09:17
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.

6 participants