New answers tagged

4

Removing a public method is not "refactoring" -- refactoring is changing the implementation while continuing to pass existing tests. However, removing an unneeded method is a perfectly reasonable design change. TDD draws this out to some extent, because in reviewing the tests, you may observe that it's testing an unneeded method. The tests are driving your ...


1

In fact f() replaces all calls to b() with the exception of the unit tests that defined / described b() The TDD cycle is very clear: write failing tests for f() (probably based on the tests for b()): tests go "Red" implement f() -> tests become "Green" refactor: -> remove b() and all tests for b() For the last step, you might consider to remove b() ...


2

Yes, it is. The best, most bug-free, most readable code is the code that doesn't exist. Strive to write as much non-code as possible while fulfilling your requirements.


6

Yes, of course. The easiest code to read is that which isn't there. That said, refactoring generally means improving the code without changing its behavior. If you think of something that improves the code, just do it. There's no need to fit it into some pigeon hole before you're allowed to do it.


2

In any one test, this can be either a good or a bad thing. Most likely, it's a good thing that you can write this test at all, and it's a good thing to have a test that runs the real CreatePerson alongside the real UpdatePerson. In general, the fact that you can use a piece of production code in tests at all is already a good thing. There absolutely should ...


1

It is perfectly ok to use production code for your tests as long as you don't shortcircuit the test. For instance, don't do this: var expected = 4; var actual = Calc.Subtract(Calc.Add(expected, 1)); Assert.AreEqual(expected, actual); As long as you know the test fails when anything is wrong, I think it is even better to reuse production code for test ...


10

There are some posibilites, how to mock static methods in PHP, the best solution I have used is the AspectMock library, which can be pulled through composer (how to mock static methods is quite understandable from the documentation). However, it's a last-minute fix for a problem which should be fixed in a different way. If you still want to unit test the ...


4

First, I would suggest to split this up into separate methods: public function validate($value, Constraint $constraint) { $totalCount = QueryTotal($value); ShowMessageWhenTotalExceedsMaximum($totalCount,$constraint); } private function QueryTotal($value) { $searchEntity = EmailAlertToSearchAdapter::adapt($value); $queryBuilder = ...


3

tl;dr Don't write unit tests. Write tests at a more appropriate level. Given your working definition of refactoring: you don't change what your software does, you change how it does it there is very wide spectrum. On one end is a self-contained change to particular method, perhaps using a more efficient algorithm. On the other end is porting to ...


2

Your certainly on the right track. For my projects I use: "check" to unit test all method (including as many code paths as I can - have time for). This runs super fast and gives me confidence that the parts of my application are doing what I expect. "Valgrind" to check the memory usage of the final application while running system/regression tests. This is ...


0

You should assert your code in the test method and avoid writing to output window. Consider adding a breakpoint instead for debugging your tests.


1

Don't waste time writing tests that hook in at points where you can anticipate that the interface is going to change in a non-trivial way. This is often a sign that you are trying to unit-test classes that are 'collaborative' in nature - whose value isn't in what they do themselves, but in how they interact with a number of closely-related classes to produce ...


2

Here my approach. It has a cost in terms of time because it's a refactor-test in 4 phases. What I'm going to expose may suite better in components with more complexity than the one exposed in the question's example. Anyways the strategy is valid for any component candidate to be normalized by an interface (DAO, Services, Controllers,...). 1. The interface ...


1

Your first task is to try to come up with the "ideal method signature" for your tests. Strive to make it a pure function. This should be independent of the code that is actually under test; it is a small adapter layer. Write your code to this adapter layer. Now when you refactor your code, you only need to change the adapter layer. Here is a simple example: ...


13

I suggest - if you haven't already - reading both Working Effectively With Legacy Code as well as Refactoring - Improving the Design of Existing Code. [..] The problem it appears to me is that when I do refactor then those tests will break as I'm changing where certain logic is done and the tests will be written with the previous structure in mind ...


5

Don't write strict unit tests where you mock all the dependencies. Some people will tell you these aren't real unit tests. Ignore them. These tests are useful, and that's what matters. Let's look at your example: public class MyDocumentService { ... public List<Document> findAllDocuments() { DataResultSet rs = ...


37

The recommended practice is to start with writing "pin-down tests" that test the current behaviour of the code, possibly including bugs, but without requiring you to descend into the madness of discerning whether a given behaviour that violates requirements documents is a bug, workaround for something you're not aware of, or represents an undocumented change ...


3

As you say, if you change the behaviour then it is a transformation and not a refactor. At what level you change the behaviour is what makes the difference. If there are no formal tests at the highest level then try and find a set of requirements that clients (calling code or humans) that need to stay the same after your redesign for your code to be ...


0

Your update asks is this sufficient to test your scenario: _officerManager.UpdateHissOfficersFromConfirm(); _sourceRepository.Verify(mock => mock.GetAllActiveOfficers(), Times.Once()); _targetRepository.Verify(mock => mock.Add(It.IsAny<List<Officer>>()), Times.Once()); Ultimately it's up to you, however I wouldn't be happy with it as a ...


52

You're looking for tests that check for regressions. i.e. breaking some existing behaviour. I would start by identifying at what level that behaviour will remain the same, and that the interface driving that behaviour will remain the same, and start putting in tests at that point. You now have some tests that will assert that whatever you do below this ...


4

Your unit tests are in a separate project and serve a separate function from your main code, so putting them into a separate namespace makes the most sense to me. If you're considering putting them into the same namespace just to save the using line, then don't. Less code is good, clearer code is better.


0

There is nothing complicated here. You just need to check if side-effect happened. Pseudocode: PreviousOfficers = _targetRepository.GetAllOfficers() UpdateHissOfficersFromSource() NewOfficers = _targetRepository.GetAllOfficers() AddedOfficers = NewOfficers except PreviousOfficers ActiveOfficers = _sourceRepository.GetAllActiveOfficers() ...


5

Do you inject the target repository (i.e. are you using dependency injection) If so, you should be able to assert that _targetRepository.Add(officers) is called by querying your mocked target repository. This is a common problem and pattern for a solution in testing. Dependency injection helps you solve this since your test can provide the mocked ...


-1

Why not return the List instead of void and that list to _actionOfficerRepository in the Confirm() method (I don't know what is calling this method). When your return type is the List instead of void it will be much easier to test this + it will be very handy for reusing this method from other actions.


4

Full disclosure: I am one of the participants in the discussion. The repository's master branch is not it's master branch. The merge into master does not serve any "actual purpose" and that branch is not doing things a branch should do (namely move). You're abusing this branch as a Tag to the latest release. Instead of using a branch, use a Tag. When ...


10

What I'd do in this situation is to mark the failing tests as "ignored" - that way you still have the test so that you know what you need to fix in future, but you're not going to end up with broken builds. If you also tag each test with the issue tracker reference for fixing the issue, that gives you an easy way to tie things together.


1

Unit tests during code review are a poor substitute for unit tests during development. What you're suggesting makes a lot of sense, intuitively. What's the review for? To check that the code is good. What are tests for? To check that the code is good. So why not combine the two? Here's why. Bringing code under test is hard work. Writing code that just ...


2

No, don't do it. You'll make them think TDD is horrid. I think @k3b has it right in the comments on the question. Code written through a TDD-style process tends to look, and interact, very differently to code written without tests. Adding (good) tests to untested code usually takes a lot of refactoring the code to clarify its intent and moving parts. By ...


1

It depends what you are doing in code review. I think there are two main reasons for writing tests at that stage: first, if you do also refactoring during code review, and you note there are not enough unit tests to cover the kind of refactoring you want to apply, add such tests second, if the code looks to you as if it might have a bug and you want it ...


22

This is a wonderful idea, with one caveat. Don't replace developer written tests with reviewer written tests. Have your reviewers look for corner cases and inputs that will break the code. In other words, have them try to write new tests that the original developer didn't think to write. Writing characterization tests is an absolutely wonderful way to gain ...


7

Wouldn't it be beneficial to write tests during code review, by the person doing the review? I have found that a good time to write tests is when you realize you need a test for a situation. Task switching for computers is expensive - even more-so for humans. At this point in time, you generally have a good understanding of the requirements and ...


6

I agree with @RobbieDee's answer but I have a bit more to add. If you really like this idea, why not have the same people write the tests before the code as executable acceptance criteria for the user story? That would do the same thing, still keep the feedback short and get everyone to have a discussion around the story ,which I think would be of greater ...


3

Like you say, if you're running a TDD team, then this is moot since the code should already be tested. Overall I don't think this is all that great an idea, but it depends on your current approach and what works for you. Basically, the problem I see is that you lose the "short feedback loop" advantage of tests. Getting instant notification the moment you ...


18

I don't think the idea is entirely without merit - however, the main benefit of the TDD et al is that problems are found early. The developer is also best placed to spot which corner cases may require specific attention. If this is left until the code review, then there is a risk this knowledge could be lost. Writing tests during the code review would ...


-5

If you have to ask, the answer is yes. Suppose some FNG comes along and thinks he can "improve" your regex. Now, he's a FNG, so automatically an idiot. Exactly the kind of person who should not touch your precious code under any circumstances, ever! But maybe he's related to the PHB or something, so there's nothing you can do. Except you know the PHB is ...


3

Regular expressions are code along with the rest of your application. You should test that the code overall does what you expect it to do. This has several purposes: Test are runnable documentation. It clearly demonstrates what you need the code to do. If it is tested it is important. Future maintainers can be certain that if they modify it, the tests ...


1

In short, you should test your application, period. Whether you test your regex with automated tests that run it in isolation, as part of a bigger black box or if you just fiddle around with it by hand is secondary to the point that you need to make sure it works. The main advantage of unit tests is that they save time. They let you test the thing as many ...


21

Regex can be a powerful tool, but it is not a tool you can trust to just still work if you make even minor changes to complex regexes. So create lots of tests that documents the cases that it should cover. And create lots of tests that documents cases it should fail, if it is used for validation. Whenever you need to change your regexes you add the new ...


99

Testing dogmatism aside, the real question is whether it provides value to unit test complex regular expressions. It seems pretty clear that it does provide value (regardless of whether the regex is part of a public interface) if the regex is complex enough, since it allows you to find and reproduce bugs and prevent against regressions.


-2

On the other hand: they themselves are seldom part of the interface of some unit. It might be better to only test the interface and do that in a way that implicitly tests the regexes. I think with this you answered it yourself. Regexes in a unit are most likely an implementation detail. What goes for testing your SQL probably also goes for regexes. ...


0

This isn't true. It's much better to place your unit-tests along side the production code when the production code especially when the production routine is pure. If you're developing under .NET for instance, you could put your test code in the production assembly, and then use Scalpel to remove them before you ship.


0

The software community is divided on the matter of unit tests. I recommend that you don't write unit tests, they are mostly useless. They are written because "it's good practice", "mature teams/developers write unit tests" or some other industry dogma. Integration or end to end tests are on other hand a sign of the team really caring about quality. ...


0

Your method GetHousesWithinSameDistrict makes use of a house repository, and an address repository, and has its own logic to select amongst the entities returned by these repositories. As such I would suggest the simple solution is to make use of mocking, and mock those repositories to return entities of the appropriate type, such that you can test your ...


0

The simple answer is: use reflection. For example, the following code creates an Address object and sets District: var address = new Address(); typeof(Address).GetProperty("District").SetValue(address, "A district", null); How you do that in your test code depends on what _addressRepository is (eg, is it declared as an interface?) and how easily it is ...


0

Unit Tests and Integration Tests are orthgonal to each other. They offer a different view on the application you are building. Usually you want both. But the point in time differs, when you want which kind of tests. The most often you want Unit Tests. Unit tests focus on a small portion of the code being tested - what exactly is called a unit is left to ...


2

If you encapsulate all IO operations in a small class with an interface and inject it to the archiving class you can mock calls to the IO system during testing. public class Archiver { public IFileSystem Filesystem { private get; set; } public void DoWork() { //business logic here } public Archiver() { Filesystem = new ...



Top 50 recent answers are included