Andy Kirkham replied to the topic 'Hide default INDI properties' in the forum. 8 years ago

Is your code in a repo such as github to look at?

As here is another example: github.com/indilib/indi/blob/master/libi...perfectstar.cpp#L136

Hard to tell what's going on without seeing more context

Read More...

Andy Kirkham replied to the topic 'INDI Testing Framework' in the forum. 8 years ago

Gerry

Why do you keep bring up the vendor binary? I keep saying we shouldn't even be loading their binary let alone testing it!

What I am saying is THE CODE YOU WRITE should be tested.

If your driver needs to do some math to move a scope from point A on the sky to point B on the sky I want to know your math is correct and the result of your calculation is a slew command.

Given point A and point B I EXPECT your driver to issue COMMAND foo{...} to the scope. Test complete. That's it.

Now, there maybe bugs in the driver that mean it does something un-expected and you need to adjust the command that is sent to compensate for it. Now that's all this reverse engineering you keep going on about. Personally I would write to the manufacture to get the bug fixed upstream. Failure to fix means no further development and a big public notice on the Indi Website telling people to avoid the buggy product.

Whatever "punishment is deemed fit", that's just an example. So people may prefer the hard life of fixing these buggy vendor binaries, that's entirely down to you. I personally probably wouldn't, I would just do my homework and buy something recommended and known to work in the first place.

When it comes to unit testing I just want to know you wrote the code to the specification you say you are working to, even if that's one adjusted for the failings of manufacturers.

I am in no way advocating fixing commercial stuff in open software, quite the opposite in fact, name and shame them into compliance would be my moto.

What I do expect is OUR CODE to work as we expect. I don't care about vendor binaries. If you do, carry on sweating over it by all means. It's your time.

Unit testing is all about OUR CODE. No one elses.

I really don't understand why you keep ing this issue about vendor binaries at all. They are totally irrelevant to unit testing our own code. Imagine for one second if I manufactured domes and I implemented "DROP TROUSERS" as the serial command to open the roof what I want your driver to emit is that text. All I have to test is the driver emits that. I don't have to stand there waiting to see if the roof opens. I know for a fact my driver is correct and compliant.

If however my driver does nothing despite I said it would open for that command I fully expect an email from you complaining. No response from me gets talked about in social media, in forums, endlessly visible for decades to come to a wealth of future buyers. My reputation plummets and I sell nothing.

But so long as your driver emits the command correctly your unit test will pass. YOU and your code are compliant. That's what unit testing is all about.

This constant talk about having to reverse engineer broken drivers is totally off topic and nothing to do with testing our own code. If they are that bad just don't support them.

Read More...

Andy Kirkham replied to the topic 'INDI Testing Framework' in the forum. 8 years ago

it's very difficult, and beyond the scope of most folks involved in this project, to create a test jig that includes the vendor provided binary code

The point about Unit Testing is you exclude the vendor provided binary. Once again, we are testing your code, not the binary.

For example, a binary has an exported function MoveTo(double RA, double DEC); Now, your driver at some point in it's life (in some function you write) will call that driver function. What we are testing is your driver makes that call with the correct RA and DEC arguments as expected given the inputs and the state. We do not call the binary at all, we don't even load it.

It's not impossible for most people to create what I am talking about. It's actually much more simple than you might think. But yes, if you have never seen this before it looks alien first time you see it. But that's just learning, we all learn as we move along.
This was why I originally started writing device simulators when I first tackled updates to the indi project. At least with the simulators, we have what should be a 'known good' piece of code pretending to react like a physical hardware device, and allows for end to end testing of the system from client thru to indi server and down thru base classes, but excluding the various hardware specific layers

Using simulators can test the clients, it can test the base class and the derived indidriver class makes the correct callback but it cannot test your driver. For example:-

indi/libindi/drivers/dome
  • baader_dome.cpp
  • dome_simulator.cpp
  • roll_off.cpp
In your model we test indi Core, Indi derived driver subclass and we finally test dome_simulator.cpp
If we were to run code coverage analysis which two files pop out from that list as untested? That's right the two we actually want to test and the one we cover we don't care about because it's only a simulator that will never drive anything other than a client integration test.

That's the difference here, you describe system wide integration tests (and not actually testing the driver unless you own the hardware). So in your world this type of testing falls short because the code you wrote in those drivers is not excerised by your test suite.

In order to test those missing "real" drivers you need to introduce two things, dependency injection which pulls in the second thing, mockable abstraction. You stated earlier these "test jigs" are "very hard and beyond most folk". They are not once you understand the concept and see it in action, it's actually pretty simple.

Simulators are only useful for those without the hardware to "get going" until the hardware they ordered arrives or for client developers who don't have the hardware. For driver writers they are useless for automated testing because we don't want to test the code in the simulator, we want to test your real code in your real driver.

You may believe that that code is really complex and hard to test. I disagree. Yes, the driver logic may well be complex itself. But when it calls IDSet*() in core that just spits out "something" to stdout. That's all you need, the ability to capture that output so you can analyse it against an expectation of what should be being sent to stdout for a given test.
I guess I _could_ spend an inordinate amount of time on a libusb hook set to test inputs and outputs for one of those cameras

Not so, I know for a fact I can mock both Indi's hid_device AND libusb in an evening. Job done, and done once, reuse MANY times for all test. The key that takes the time is refactoring your to use the dependency injection in the drivers and then retrospectively designing suitable tests for legacy code. It's those tests for the legacy existing code that will take time for sure. But what's the rush? Introducing the DI won't break anything, the driver will still function perfectly without a single test. But it doesn't have to be done in one go. Do it one driver at a time and one driver API function at a time.
In the process of doing this project, one thing I came across is the ascom conform test suite, far from perfect, it attempts to reach some form of testing. It appears to be a client program that will exercise a device testing for inputs and expected outputs. It is essentially a black box form of testing. I think indi could benefit greatly from something along those lines as a starting point for automated tests. what I would envision for that, is something along this line.

This is what I am advocating. The difference is rather than one big huge client that tests the entire world you break it up into unit tests that exercise API functions. So here is my question. You say "testing for inputs and outputs". That's what I am saying. You control teh inputs, that's easy. Exactly how are you going to get those outputs? In my world that's where the DI and mocks come into play. It makes it simple. But essentially we are talking about the same functionality here, just now discussing how to actually do it. Just trying to redirect stdout will only take you so far. Mocks provide the ability to return values which in other ways you would need to try and use stdin. stdin/stdout redirection is way to messy. Mocks are perfect, they were designed for it, specifically Google Mocks in Google's GTEST suite.
The way the ascom folks do it, when I run the conform tests on the dome driver, final output is a test sheet showing pass/fail on each of the tests

That's exactly what Google Test will do for a pass. For a fail however, you get a ton of extra information about the nature of the failure. Not just a "fail, game over, you lose" which is pretty much all integration tests can tell you.
Unstable - code builds in the test environment, drivers load - no functional tests completed (no hardware present for testing), ie it has passed the first set of automated tests in terms of building and loading.

Testing - code builds in the test environment, drivers load and function correctly against test jigs for those that have test jigs.

Stable - code builds in the test environment, and has been tested against physical hardware at a location that has the hardware

This is pretty much standard in the open source world, it's normally call, in git dev, "release tags" (stable), master branch (pre-stable), develop (unstable), other branches are down to developers and they should merge to develop when ready.
And yes, this is a very ambitious addition to the project as a whole, but, it would vault us up into a much higher level of professionalism in the development process. The real question then becomes, does anybody have the time / inclination to follow through, it's a big task. but there is a fairly strait forward direction to accomplish it incrementally, first we get core indilib set up for automated testing with just simulator drivers under test. The next phase would be to start tackling drivers on an individual bases, define how each could be rigged for automated tests, then implement.

It is ambitious yes. But we already made a start with this discussion. There are two types of testing:-

1. Unit tests. These run on every commit of code, they are automated and hold your contract to account for APIs.
2. Regression tests. These normally happen when you form a new branch from master into a release branch. Once you have a release branch you regression test it and run any automated integration tests you have (usually requires some human input). You may get bug fixes here but they have to be very minor. Major bugs cause release process abort as you are not ready. Once you release, you tag and then merge back to master and the branch can be deleted. New cycle starts.

I hope this is understandable. The key point is to test as much code as is possible. I think simulators are only useful for client and integration tests. But they don't reach the real complex driver code base at all. That requires DI and mocking.

Read More...

Andy Kirkham replied to the topic 'INDI Testing Framework' in the forum. 8 years ago

Will respond later, need to walk my dog. Great write up though and thanks for sharing. We are getting closer to a realisation of direction :) Please remember, I'm new so much of the experience you are sharing is all new to me. I am coming from a different angle but I believe we are on the same page.

Read More...

Andy Kirkham replied to the topic 'INDI Testing Framework' in the forum. 8 years ago

knro wrote:

But I'm not going to battle with on this. If you don't want automated tests then fine, I'll pass on this and not waste my time.

I don't think we should pass the opportunity for automated testing. However, we should approach this carefully and progressively. How about a middle ground approach? A lot of issues, like the issue that brought up this very subject, was due to a bug in the core library which automated sets would certainly detect. How can we implement this without a complete rewrite of several of the core components?

Progressively for sure. It's not going to happen overnight to the entire codebase. It'll have to come bit by bit. And I don't expect all INDI developers to jump on it right from the start. I will agree that having UTs in place can be something of a bind to try and start doing right from the start. It has to be a gradual process and most likely led and championed by someone. But that someone (or team) have to promote it and educate where required.

In my conversations with Gerry I offered to look at his driver. Then he swapped from that to another. So, lets not dance on this. What I suggest is this. Introduce the base framework. And then write one UT specifically at the bug this issue raised in the first place and put that in place. Once we have that we can come back around the table and decide exactly how you want to proceed. Maybe UTs for Core but not drivers? Or drivers can get UTs if their maintainer wants them? There are so many routes to take but that can be decided as we progress.

So this evening I will look for that bug you fixed (from the original issue) and UT it. Then we take it from there.

However, I would like to respond to Gerry's last post where he raised some legitimate concerns:-
so the question I've been trying to get answered, but obviously not clearly, how are we supposed to build something around the vendor binary to test an all up driver?"

The vendor binary is a shipped driver with an exported API. What happens when you call that API is of no concern what so ever to you. We are talking about testing your code, not their binary. However, this doesn't answer your question at all because your expectation is that something in the real world happens when your code does something. This is where you are missing the point.

Lets explain this by example which I hope is clear.

First, an ideal situation that doesn't happen very often but we all aspire to. Lets imagine for a second a vendor ships a binary driver and a PDF document that describes the function calls it exports that you consume. Lets assume this documentation is perfect and the driver is bug free. Let's also assume it's simple, opens and closes a dome roof. One function "root(arg)", one arg, "open" or "closed".

The natural instinct here is to write a program that opens and closes the roof. It's simple right? But will that work at my house? Don't know, I don't own one to try it on. This type of testing only works for you and others who own one. But who cares, why would I want to know? I don't own one. But the point here is testing is limited to only those who own one. No possibility of automated tests here except in a severely limited environment. But like I said, who cares?

So, now lets move onto a real world case. One we all experience everyday. Same as above but for whatever reason the vendor now believes the building moves and the roof stays still. They think "open" is now "closed" and visa versa. So, your expectation of this specification is easy but when you try to implement it then it fails. Is this a failure in your expectation? Or is it a bug from the vendor? Who knows, all we do know is the outcome is unexpected behaviour. Something needs to be fixed. So, after some investigation you change the way you call their API and you get the expected outcome. You may even leave a comment in the code lambasting the vendor. But who cares, it's fixed.

Six months later Johnny buys a dome, installs it and fires up your code. And it goes the wrong way! Little does Johnny know he's wired it back to front. But hey! Look, the software is backwards, he fixes it and commits. His dome works. Two weeks later you upgrade and..... Now, clearly it would have been better if Johnny hadn't done that. That's where unit tests come in. Swapping the fictionality would have "broke the build" and his commits would be pulled pending further investigation. Now yes, Johnny could have also smudged the unit tests to pass also. But the important point here is the unit tests are the specification. Change them at your peril! That's TWO mistakes if you do that. When the build breaks stop! What have I done?! is what is scream at you because a failed unit test is a specification/API breakage.

The above is very simplistic. It's more likely Johnny will find a "almost suitable" function and bend it slightly to meet his needs but breaking your in the process. Unit tests are your contract that in future the underlying code is sound and meets the specification.

Also, it's worth noting on how you build with unit tests. There are those out that that preach TDD (writing tests BEFORE you write code). I actually suck at this myself. I'm like "I don't know what I want to test yet, I need to write some code first!". That is not true TDD. But my take on it is if you commit UTs along with the code together you did TDD. It's tested.

I tend to achieve this by being in one of two states during development.

State 1, "the clean designer". Here, the spec is clear and the vendor API is fault free. Implement something, test it, glue it with a unit test, repeat (until done).

State 2. "the oil stained engineer". Here, State 1 was going great but then suddenly the vendor's API behaved in an unexpected manner. STOP! As an oily engineer I swap hats and start an investigation (which often involves writing spike code which you throw away). The outcome of which clarifies the specification (whether that be talk with the vendor or more often than not, reverse engineer). Once you have a clear understanding of the spec again swap hats back to State 1, implement, test, glue it in place with a unit test.

That tends to be my workflow. The unit tests that "glue it in place" are a true reflection of your final specification (after fixes if needed). Your working code is the implementation of that specification.

Lets come back to one more point. Imagine Johnny changed your code and committed it. Johnny doesn't like TDD, not only did he not fix the UT to match his change, he didn't even bother to run the test suite at all. Now, if that change lands back with you in a future upgrade you don't notice then that swap could actually damage your dome (and maybe a telescope?). You are going to have to pay for that! And the only way anyone will catch this bug is when your roof hits your telescope. Ouch. It doesn't get more real world than that. Unit tests would prevent that.

It all works ok if it's only you, your code, your dome and your telescope. But the moment you not only make it public it's going to end up one day running on someone elses system. Community code needs a level of quality that people trust.

You are already critical of SBIG and QHY quality of code. If you want to be publicly critical of others then it's best your own house is in order first. Lead by example.

Maybe the INDI website should actually start publicly listing vendor specification departures or plain deaf to support requests. We have an opportunity with a project like INDI to not only provide some levels of QA of INDI but to lead by example and shame some vendors into a more positive light. Lifting our own coding standards lifts those around us as well :)

Everyone wins (except of course unit tested code takes long to do because simply it's more work but the long run benefits outweigh the short term "feel good it works now" factor).

Read More...

It doesn't matter if you forward or reverse engineer anything. The engineering aspect is always the same. You need to figure stuff out. Whether it's solve new problems of your own or solve other peoples problems in reverse engineering it's all the same, your eventual output is code.

And yes, I live and write code for the real world too. I've written code for automated trains, for aeroplanes, etc. They all move slower around the World just like a dome or scope. The point is automated testing is about testing the code you end up writing as a result of your engineering efforts, whether that effort is based on forward or reverse engineering.

Your simulators are manual. You, a real person in the real world, has to click stuff. And if you want automated tests then you will need to automate those simulators. Congrats, you just re-invented a wheel that's already been solved elsewhere.

And as for timing, like a dome or scope moving in real time. You miss the point. You mock the clocks too so your software believes it took 10 seconds for something to happen but in fact was a couple of microseconds. If automated tests had to wait real world time nothing would ever build because new commits will land in teh repo before the last build completes! Mocks are the key.

But I'm not going to battle with on this. If you don't want automated tests then fine, I'll pass on this and not waste my time.

Read More...

ok, I will go look at your code and have a ponder.

But, here is teh really important bit I think you are missing. You are NOT testing the hardware in the corner of the room. That's someone else's job (the manufacture).

You are testing YOUR CODE. So, as long as you code emits the correct data that would have gone to the mount your code passes the test. No need to watch a mount actually slewing at all. You just have to ensure YOUR CODE (under test) emits the correct command with expected data.

That's where the abstraction comes in. We need to capture that command as it leaves and check it's correct.

If you use tty_write() then it's gone. No chance to check it. But if we wrap around tty_write() we can capture WHAT YOUR CODE sent and check it was what we expected should be sent under that condition. That's what the swappable interface gives you.

Remember, we are testing code, not hardware. Travis-CI doesn't have your mount, a dome, a CCD, etc etc etc/all possible hardware. So we need to monitor the IO of what the code is doing against an expected set of preprogrammed outcomes.

For the most it can be somewhat tiresome I agree. However, think like this. Imagine you had a full unit test suite in place from the start of your project. Then, in 2 months someone reports a bug. In your world only you can fix it because only you have the mount. In the CI world what you do it write a unit test that recreates the bug actually happening. The unit test will fail. Fix the bug and the unit test will now pass. And for all time to come that unit test will be there to prevent a regressive bug coming back because it runs every build.

I will go look at your code as my first UT target (I was going to do something simple like a dome, but hey, lets get real world and demonstrate this)

Read More...

Gerry,

It's not a second layer. Your first layer you pointed to are "helper functions", not a replaceable interface. All calls to, for example, tty_connect() will do exactly what that function does. There's no opportunity to swap out tty_connect() with an different function (mock).

The Mock is placed around your helper functions. Then, using Google Mocking it's possible to swap out one interface for another. Google Mocking is an advanced mocking system that allows you to use EXPECT(TIMES()), RETURN(whatever) macros.

I will say if you have never seen or used it then yeah, it's a bit alien. But once you see it in action you'll understand more.

Read More...