-
Notifications
You must be signed in to change notification settings - Fork 1
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
Refactoring Unit Tests #35
base: main
Are you sure you want to change the base?
Conversation
Hi @nickmartin1ee7, what do you think about the progress so far? |
I'm planning to:
What's your opinion about it, @nickmartin1ee7? |
Progress have been make. Did I do it right? @nickmartin1ee7 @DonSagiv |
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.
There are a few issues here noted in my comments.
I like the idea of a static class that removes the issue with identical fakes being instantiated in every test though.
/// <summary> | ||
/// Represents a fake <see cref="ProjectDAL"/>. This class cannot be inherited. | ||
/// </summary> | ||
internal sealed class FakeProjectRepository : ProjectDAL { } |
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.
You should not inherit the DAL. The DAL will be the actual connect to a database. A fake repository should just inherit IProjectRepository
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.
I inherited that Fake from ProjectDAL just to reuse the code, but now should I copy and paste the code of the ProjectDAL to the Fake?
I just want to point out how easy it was to apply these fixes, I just had to change a few lines in TestConfig. I got that idea from: Writing Clean Tests – It Starts From the Configuration. |
@srjheam Would you want to update this branch, and look at this again issue again? |
As previously discussed in #30 and #29 refactoring this project's tests might be a good idea
Closes #30