Implementing GetProject() with lambda calls#107
Implementing GetProject() with lambda calls#107WiseLordship wants to merge 3 commits intoQualiSystems:masterfrom
Conversation
borismod
left a comment
borismod
left a comment
There was a problem hiding this comment.
@WiseLordship thanks a lot for your contribution. I'll appreciate if you could take a loot on my questions.
| A.CallTo(() => teamCityCaller.Get<Project>(expectedCall)).MustHaveHappened(); | ||
| } | ||
|
|
||
| private static TeamCityCaller CreateTeamCityCaller() |
There was a problem hiding this comment.
Why not reusing CreateTeamCityCaller from AcceptanceTests?
There was a problem hiding this comment.
I started with that and realized I was not mocking it the right way. So moved that out and new tests should be following the mocking correctly.
FluentTc/Engine/ProjectsRetriever.cs
Outdated
| } | ||
|
|
||
| public Project GetProject(string projectId) | ||
| public Project GetProject(Action<IBuildProjectHavingBuilder> having = null) |
There was a problem hiding this comment.
Why having is nullable? What is the expected result when we call GetProject() ?
There was a problem hiding this comment.
You are right, I was not fully looking at it correctly. I added the logic to make sure we are only returning one and made it similar to GetAgent().
| [TestCase("FluentTC")] | ||
| [TestCase("Test 1")] | ||
| [TestCase("Test%20C")] | ||
| public void GetProject_ByName(string projectName) |
There was a problem hiding this comment.
Does this test covers URL encoding?
There was a problem hiding this comment.
I was actually just testing random characters and thought that would be the most common that someone would enter. Currently, not handling URL encoding.
| namespace FluentTc.Tests | ||
| { | ||
| [TestFixture] | ||
| public class ProjectRetrievalTests |
There was a problem hiding this comment.
These are acceptance tests and they should reside inside AcceptanceTests
There was a problem hiding this comment.
I will move these to AcceptanceTests and will make better overall tests for the method. I started here when I was trying t make more robust tests, and did not move these out.
|
@WiseLordship what is the status of this PR? |
2 participants
Issue details
Implementation for getting projects based on lambda expressions.
Relates to issue
#102
Checklist
Example of using new/modified functionality