patterncsharpMinor
Is this UnitTest for updating an object in data-access layer sensible?
Viewed 0 times
thislayerupdatingunittestsensibleforobjectdataaccess
Problem
I'm still trying to learn how to write good tests (and write testable code).
This is the function I want to verify,
And the unit test:
```
[TestMethod]
public void UpdateRange_RangesPropertiesAreCopied()
{
var rangeToUpdate = new Range()
{
Id = 1, CourierId = 0, Status = 20,
WorkflowServer = null,
FirstPsn = -1, LastPsn = -1, PageCount = 0
};
var rangeToSave = new Range()
{
Id = 1, CourierId = 1234, Status = 40,
WorkflowServer = new PhysicalStation(){ Id = 1, Name = "fakeserver"},
FirstPsn = 123, LastPsn = 456, PageCount = 333
};
var repoMock = new Mock();
repoMock.Setup(x => x.Query()).Returns(new List(new[] {rangeToUpdate}).AsQueryable()).Verifiable();
repoMock.Setup(x => x.Commit()).Verifiable();
var factoryMock = new Mock();
factoryMock.Setup(x => x.GetRepository(It.IsAny())).Returns(repoMock.Object);
IAutoTisDal dal = new AutoTisDal(factoryMock.Object, "");
dal.UpdateRange(rangeToSave);
repoMock.VerifyAll();
Assert.AreEqual(rangeToUpdate.Id, rangeToSave.Id);
Assert.AreEqual(rangeToUpdate.Co
This is the function I want to verify,
void IAutoTisDal.UpdateRange(Range range)
{
using (var repo = _repositoryFactory.GetRepository(_baseline))
{
var rangeqry = from r in repo.Query()
where r.Id == range.Id
select r;
var rangeToUpdate = rangeqry.SingleOrDefault();
if (rangeToUpdate == null)
throw new Exception(string.Format("Range ID {0} not found", range.Id));
rangeToUpdate.Status = range.Status;
rangeToUpdate.FirstPsn = range.FirstPsn;
rangeToUpdate.LastPsn = range.LastPsn;
rangeToUpdate.PageCount = (int)(range.LastPsn - range.FirstPsn);
rangeToUpdate.CourierId = range.CourierId;
rangeToUpdate.WorkflowServer = range.WorkflowServer;
rangeToUpdate.DoubleFeedInd = range.DoubleFeedInd;
repo.Commit();
}
}And the unit test:
```
[TestMethod]
public void UpdateRange_RangesPropertiesAreCopied()
{
var rangeToUpdate = new Range()
{
Id = 1, CourierId = 0, Status = 20,
WorkflowServer = null,
FirstPsn = -1, LastPsn = -1, PageCount = 0
};
var rangeToSave = new Range()
{
Id = 1, CourierId = 1234, Status = 40,
WorkflowServer = new PhysicalStation(){ Id = 1, Name = "fakeserver"},
FirstPsn = 123, LastPsn = 456, PageCount = 333
};
var repoMock = new Mock();
repoMock.Setup(x => x.Query()).Returns(new List(new[] {rangeToUpdate}).AsQueryable()).Verifiable();
repoMock.Setup(x => x.Commit()).Verifiable();
var factoryMock = new Mock();
factoryMock.Setup(x => x.GetRepository(It.IsAny())).Returns(repoMock.Object);
IAutoTisDal dal = new AutoTisDal(factoryMock.Object, "");
dal.UpdateRange(rangeToSave);
repoMock.VerifyAll();
Assert.AreEqual(rangeToUpdate.Id, rangeToSave.Id);
Assert.AreEqual(rangeToUpdate.Co
Solution
First of all, your single method is doing multiple things:
Let's try to separate responsibilities:
Two new methods are already easier to understand and test.
And, honestly, I don't think that writing test for IAutoTisDal.UpdateRange makes much sense. It won't provide much value.
- Get Range by id
- Copy properties from one instance to another
- Commit
Let's try to separate responsibilities:
Range GetRangeById(Repository repo, int id)
{
return repo.Query().SingleOrDefault(r=>r.Id == id);
}
void UpdateRange(Range rangeToUpdate, Range range)
{
// I'd even make this Range extension method
rangeToUpdate.Status = range.Status;
rangeToUpdate.FirstPsn = range.FirstPsn;
rangeToUpdate.LastPsn = range.LastPsn;
rangeToUpdate.PageCount = (int)(range.LastPsn - range.FirstPsn);
rangeToUpdate.CourierId = range.CourierId;
rangeToUpdate.WorkflowServer = range.WorkflowServer;
rangeToUpdate.DoubleFeedInd = range.DoubleFeedInd;
}
void IAutoTisDal.UpdateRange(Range range)
{
using (var repo = _repositoryFactory.GetRepository(_baseline))
{
var rangeToUpdate = GetRangeById(range.Id);
if (rangeToUpdate == null)
throw new Exception(string.Format("Range ID {0} not found", range.Id));
UpdateRange(rangeToUpdate, range);
repo.Commit();
}
}Two new methods are already easier to understand and test.
And, honestly, I don't think that writing test for IAutoTisDal.UpdateRange makes much sense. It won't provide much value.
Code Snippets
Range GetRangeById(Repository repo, int id)
{
return repo.Query<Range>().SingleOrDefault(r=>r.Id == id);
}
void UpdateRange(Range rangeToUpdate, Range range)
{
// I'd even make this Range extension method
rangeToUpdate.Status = range.Status;
rangeToUpdate.FirstPsn = range.FirstPsn;
rangeToUpdate.LastPsn = range.LastPsn;
rangeToUpdate.PageCount = (int)(range.LastPsn - range.FirstPsn);
rangeToUpdate.CourierId = range.CourierId;
rangeToUpdate.WorkflowServer = range.WorkflowServer;
rangeToUpdate.DoubleFeedInd = range.DoubleFeedInd;
}
void IAutoTisDal.UpdateRange(Range range)
{
using (var repo = _repositoryFactory.GetRepository(_baseline))
{
var rangeToUpdate = GetRangeById(range.Id);
if (rangeToUpdate == null)
throw new Exception(string.Format("Range ID {0} not found", range.Id));
UpdateRange(rangeToUpdate, range);
repo.Commit();
}
}Context
StackExchange Code Review Q#18541, answer score: 5
Revisions (0)
No revisions yet.