Comments (8)
I have all the test failures fixed locally, but have two issues.
One: I'm unfamiliar with StructureMap (guess I should go back and read @chrissie1's posts) and the correction for several of the tests was to switch the constructors from using locally instantiated concrete objects (mostly Run and RunAsync) to use the provided interface parameters that would be supplied via IoC. So while the tests now pass, I'm less sure about how the code actually runs, as I don't see anything obvious that would tell it to correctly choose between the Sync and Async version of IRun objects.
Two: I'm tracking down 3 dialog boxes that appear while running the tests and have found another trail of concrete constructor calls. I can keep tracking these down, or I can try to rewire them to use the registry for StructureMap. My suspicion is that the dialogs are side effects from a concrete class constructor somewhere.
The project seems to be partially using IoC and partially bypassing it. Is there a preference? My options are that I can either continue to straighten it out in favor of StructureMap or in favor of the default constructors, but I don't want to get too much deeper into the tests without knowing which is the preference.
from chocolateygui.
I would say that we need to be consistent. If we are using IoC elsewhere in the project, then we should be using it everywhere. This will make the code more maintainable in the long run. There is another open issue to replace StructureMap with TinyIoC, so perhaps this work should be done before any further investigation. Thoughts?
from chocolateygui.
We definitly want to use IoC but structuremap can go away, I was thinking TinyIoc or Autofac. I think we need to educate @cemrich in the use of Ioc/DI and maybe add a irunsync and irunasync. I thought everything either ran sync or async but it doesn't seem to be true. Changing IoC container was issue number #11
from chocolateygui.
Ripping out structuremap and replacing with another should be simple enough. Only one registry and one place where I use the container, in program.cs (which is how it should be).
from chocolateygui.
I am familiar with the concept of dependency injection from java development and I guess StructureMap
is a way to achieve this in C#.
One class that doesn't make use of it yet is CachedPackagesService
. This one is a bit tricky because it is able to cache package lists using any IPackagesService
but it implements IPackagesService
by itself. I'm not sure how to change the architecture in a way that:
- the
PackageManager
can use a cachedIPackagesService
but does not have to - the
CachedPackagesService
can get itsIPackagesService
implementation via DI
One easy way would be to let any implementation ofIPackagesService
handle the caching by itself but this could mean a lot of duplicated code.
Regarding the tests I think it's not wise to use DI for most of the tests. The test cases are mostly white box tests at the moment. They test for internal behavior like "does method xy call method z with these arguments". These tests are highly implementation dependent and should not use DI.
However there should be tests testing functionality that doesn't depend on the concrete implementation (like IfCurrentSourceIsNotemptyWhenInstantiated
for testing ISourceService
). These black box tests should run on all available implementations of the interface they test (which maybe could be achieved using DI).
I don't understand the sync and async thing yet. As far as I can see these classes are used to communicate with powershell. @chrissie1, can you please explain this a bit further?
from chocolateygui.
To solve the problem with the cachedservice just create another interface it can be empty. It's a small hack but the world isn't perfect and I can live with it. The other way around is to do complicated IoC configuration but that just wastes time and can lead to much pain. I have found that you should not overcomplicate the IoC configuration. Better to do the simple thing and create an extra interface if you need a particular implentation of something. It also makes it clear to the reader of your code what it is you are doing when you use the interface. Lot's of configuration will lead to lots wondering why am I getting this implementation and not that one.
The DI is there mainly to make it easier to test and to mock out the dependencies to test the behaviour of the SUT not the dependencies. We don't want our unittests to have a need for chocolatey or even powershell. They should run in isolation without a need for their dependecies and test behaviour as you say.
We can and should have integration tests that go out to chocolatey but then we have to make sure that the buildserver can run them.
You can run the powershell commands in sync, wait for the result and then go on. Or run them async, don't wait for the result and go on. Either way should work. But using the Async way you have to be carefull with crossthreadexception in the view. The Async should be used as much as possible so that you don't block the viewthread.
So I would make an ICachedPackagesService that inherits from IPackagesService (if needed) and add that to the registry, make CachedPackagesService implement ICachedPackagesService and then use that.
And sorry if I insulted you, it was not my intention.
from chocolateygui.
I'm modifying things in favor of using the IoC container, it's nearly there and then I'll merge it in and let someone do the StructureMap to TinyIoC conversion. I've added IRunSync and IRunAsync, added a file system abstraction for the few places that access the filesystem to reduce their reliance on test files being placed just so (a couple of the failing tests were due to content changes in the sources file that don't have any effect on the actual logic but caused the test expectations to fail).
from chocolateygui.
The DI is there mainly to make it easier to test and to mock out the dependencies to test the behaviour of the SUT not the dependencies. We don't want our unittests to have a need for chocolatey or even powershell.
Ah, I didn't thought of that.
You can run the powershell commands in sync, wait for the result and then go on. Or run them async [...]
Okay, than it's just the powershell adapter. The name confused me because I've overlooked the namespace (I expected something like AsyncPowershelAdaper
), that's my fault.
And sorry if I insulted you, it was not my intention.
It's okay - I just have to figure out which knowledge I'm able to transfer to C# an which not. I'm sorry if that messes up parts of the code. Of course you can always say so when things can be improved :)
I'll look after the CachedPackagesService
as soon as @tarwn has finished.
from chocolateygui.
Related Issues (20)
- Upgrading to Latest 2.0.0 and 6.0.0 of Chocolatey Products
- Package version normalization is not being applied everywhere HOT 1
- Error message "Unable to resolve dependencies. 'package A' is not compatible with ..." not displayed in the GUI
- Duplicate registry keys HOT 2
- Chocolatey GUI throws repeated HttpRetryHandler warnings while attempting to update Chocolatey package list HOT 8
- Opening settings window crash HOT 2
- Chocolatey GUI should honour the proxy settings of Chocolatey CLI when downloading the icons
- Provide ability to use option for Chocolatey CLI commands to skip cached HTTP query results HOT 1
- Unexpected output in Console and Log
- Allow default sort order to be a setting in Chocolatey GUI
- Exit code 1603 when trying to install chocolateygui 1.1.0 HOT 3
- Exit code 1603 when trying to install chocolateygui 1.1.0 HOT 11
- Configure Chocolatey GUI to prompt for repository credentials like the Choco CLI HOT 1
- Unhandled Exception with GUI despite multiple un- and re-installs HOT 2
- Provide a way to ignore HttpCache when retrieving packages
- Make the package view for remote sources and This PC source consistent
- Add feature to automatically refresh the view on install, upgrade, reinstall or uninstall
- Add clearer messaging when downgrading a package HOT 1
- Failed to start application. HOT 1
- Related to #112 HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from chocolateygui.