Hot answers tagged

20

if (moduleName == null) { moduleName = componentType == vbext_ComponentType.vbext_ct_StdModule ? "Module1" : componentType == vbext_ComponentType.vbext_ct_ClassModule ? "Class1" : componentType == vbext_ComponentType.vbext_ct_MSForm ? "Form1" : "Document1"; } Sorry, that I have ...


16

Almost all of this code can (should?) be moved directly into the MockFactory. [TestInitialize] public void Initialize() { _ide = MockFactory.CreateVbeMock(); _ide.SetupProperty(m => m.ActiveCodePane); _ide.SetupProperty(m => m.ActiveVBProject); _ide.SetupGet(m => m....


13

Naming Interfaces are usually named starting with a capital I in .NET. ScientificFormatter Here you have public string Format(BigInteger number) and private string FormatNumberString(string numberString) where the second would be easier to read if you would use method overloading. Method overloading is just that you can have a method with the same name ...


13

So first, as CAD97 rightly points out, if you simply define your protocol as identically matching the methods from UserDefaults you intend to use, then making UserDefaults conform to the protocol is much simpler: extension UserDefaults: UserDefaultsProtocol {} And there ya go, done. Having to write less code is nice. What's even nicer here is that we're ...


12

In dependency injection terms, this factory class has a dependency upon a TextWriter instance. I would much prefer to see that dependency be constructor-injected: private readonly TextWriter _writer; public TextWriterFactory() : this(new StringWriter()) { } public TextWriterFactory(TextWriter writer) { _writer = writer; } That way you get rid of ...


11

Even though the SOLID principles are known to be principles related to Object-Oriented Design, I'd still take a cue from the Dependency Inversion Principle. As APPP states it (ch. 11): "clients [...] own the abstract interfaces". You can apply this principle in FP as well, in the sense that you can start by defining the overall behaviour of the function you'...


7

I really rather think you have the right idea with the interface and your real class. I do heartily endorse your use of readonly which doesn't get a lot of exposure. However, there are many good mocking frameworks out there that you don't need to write your own fake class to unit test with. For instance, we use Moq where I work and your unit test example ...


6

As far as I can understand (I use Moq as an isolation framework and not rhino mocks) in your test you are testing not an implementation of your class FillTableRow, but a mock, generated by isolation framework. You should always use your real class as systam under test (obviously). You use stubs to provide context in which your system under tests works, just ...


6

Except for some (mostly experience related issues) I commend your efforts. Well done! The Short Version Specific Points use app\models\User; is not needed as the User class is not used (or mocked) in this test. The comments before your asserts don't always seem to match what you are actually asserting. I think you'd be better of using those comments as the ...


6

This isn't a full review, it's just a couple of initial thoughts: Unlike Java, Interfaces in C# are usually prefixed with 'I', so BigIntegerFormatter becomes IBigIntegerFormatter. If you aren't committed to using the microsoft test framework, you might want to consider using something like Nunit, which supports the TestCase attribute. If you use the ...


6

It's very hard to follow the builder when it's jumping around like it is... I wonder whether you could achieve something like: var Mock<VBE> mock = new MockVbeBuilder() .AddProject(settings => { settings.Name = ""; settings.Protection = /* something... */; settings.ComponentBuilder ...


6

According to this, subclassing datetime.datetime seems the way to go. There is no use for the str_2_time method though. You can easily inline this, or even simpler, just use the datetime.datetime constructor: def test_patch_datetime(): for utc_time in [ dt.datetime(2019, 1, 1, 10), dt.datetime(2019, 2, 1, 13), dt.datetime(2019, ...


5

I see no reason not to test this class. It exposes a public method that operates upon its arguments using non-trivial logic. To decide how to test it, you must consider what the method does, step-by-step, and determine which parts are worth testing. Reading the checkAllAlternativesShort method from start to finish: It loops X times around a for-loop, based ...


5

I think that Mark's answer gives a great alternative perspective here. That said, I would probably write something that's quite similar to what you did here. There is a couple of small tricks that I would perhaps use to make the code a bit nicer (at least for my own personal idea of "nicer" F# code :-)), so I thought I'd share those. First of all, I would ...


5

While I think the underlying idea is quite nice, the actual implementation and design has some issues. Wrong Abstraction Level Let's start with the less obvious one: How would you actually use Clock or BaseClock with std::chrono::high_resolution_clock or std::chrono::system_clock? The simplest approach would be something akin to this: struct HighResClock : ...


5

Mocking system calls, especially those with side effects (mkdir()), is difficult and error prone. I would rather use a temporary directory for test purposes. This approach has the added advantage of really testing your FS manipluation code. import datetime import os import pathlib from contextlib import contextmanager from tempfile import ...


5

I'm no professional when it comes to testing. But it looks like your code is a perfect example on non-test friendly code. And is why SRP, single responsibility principle is such a big thing. From my perspective, your function is doing two things: Making path. Ensuring path exists. If you change your code so that those are two distinct functions than you ...


4

Two minor notes: The following is not too easy to read: curl_setopt($r, CURLOPT_ENCODING, 1); Readers have to be really familar with curl parameters or have to check the documentation. It says the following: The contents of the "Accept-Encoding: " header. This enables decoding of the response. Supported encodings are "identity", "deflate", and "gzip"....


3

I'm not familiar using py.test, but here are some comments I have after reading your code: There are no comments - this would be helpful to the reader. In text_parser, you have from tparse import TextParser. I would advise moving this import to the top of the file, for readability. What is your code trying to do with these tests? What are you testing?


3

You would be testing the MapMarker more in isolation if you would inject only mocks. Isolation is important for two things: stability of test results more explicit and direct feedback If a bug would be introduced in the RailsMarker, this test would also fail, causing it to be less stable. In case of this bug, there would by multiple test failures (also ...


3

Your tests looks good although I don't have any experience with JMockit (just Mockito and EasyMock) so I might miss some JMockit-specific issues. Here is a few notes about the code: Instead of String[] tokenizedURL = URL.split("/"); String word = tokenizedURL[tokenizedURL.length - 1]; You could use StringUtils.substringAfterLast. See also: Simplest way to ...


3

I decided that there was too much database access code in my UserPermissions class and split it into two classes. The first class is now much easier to mock and the second class is just a wrapper for a doctrine query, which is probably almost pointless to mock. class UserPermissions{ protected $dbalUserProducts; public function __construct($...


3

You should introduce Dependency Injection so that FillTableRow<T> receives the object of type IDbObjectFactory<T> rather than instantiates it itself. Once you've done that you can write tests by injecting stub of IDbObjectFactory<T> into FillTableRow<T>. In your current code there is no need in creating mock for FillTableRow<T> ...


3

The design of the readconfig() function is seriously problematic. It makes no sense to convert caught exceptions into data, as if the file literally contained the string "FileNotFoundError" or "PermissionError". If you don't have a good way to handle an exception, the right thing to do is let it propagate. The readfile() function should be simpler: def ...


3

First of all, Bravo for writing unit tests! Secondly, Bravo for doing it in PHP! Seriously though, I don't see enough people doing this type of testing. What you're running into is referred to as test pain. Test Pain describes the amount of effort and setup required to sufficiently isolate code dependencies in order to perform unit testing. In your case, ...


3

I think the test is ok. Remember what you're testing is this const signIn = ( username, password, successCallback, errorCallback, ) => { Auth.signIn(username, password) .then(user => successCallback(user)) .catch(err => errorCallback(err)); }; and NOT Auth. Here Auth is just a dependency so you will stub its functionality. ...


3

The thing is, there is not much to test in your code. You have added several abstractions to a simple statement: if (isValid($data)) { doSomething($data); return makeResponse(200); } else { return makeResponse(422); } So, your unit is a single if statement, and all actions -- condition, then branch, else branch -- everything is passed as ...


3

You make the code really hard to read: #define INJECTION_SET(FuncName, Substitute)\ private:\ template<class T>\ class FuncName##Class {\ public:\ FuncName##Class():m_func((T)Substitute){}\ T m_func;\ };\ FuncName##Class<decltype(Substitute)> m_##FuncName##Class;\ mutable int m_##FuncName##CallCounter = 0;\ public:\ void Inject##FuncName(decltype(...


2

The benefit of using an adapter in this context (though i'm not sure it's an adapter, it feels more like a facade, even if it just sits in front of 1 class) is that you can translate the objects of the external system to your internal model. 1: I would suggest the pattern you use is a facade. as for package, I would put it in it's own package named "XXX.job"...


2

I'm assuming you're fine with assert, generally I'd recommend using the unittest library. Now for the monkey patching the other choice you have is to restructure how the credentials are loaded. At the moment the credentials are loaded every time a request is performed - instead of that the credentials could be loaded once, for the whole module as globals, ...


Only top voted, non community-wiki answers of a minimum length are eligible