How to mock libplctag.NET

[timyhac]

@timyhac Is there a reason why this constructor of the Tag class was made internal? I would like to write test for custom Tag-classs that use the real Tag class.
And then pass a mock for the INative for local testing without a device for both the tag + any encoding or decoding.
But as the constructor is internal and the class no longer has an interface. This is now unnecessary complicated because the design ingores the open-closed principal.

I also don't understand why the ITag interface was deprecated and why the Tag class does not implement this interface. Could you please elaborate?

Originally posted by @sebastian-zitzelsberger in #406

Hi @sebastian-zitzelsberger - apologies if I am misinterpreting your query; to me it seems that the core requirement is to be able to mock the device you're connecting to - which is unrelated to #406 (hence why I've moved this conversation to a new issue).

The approach I would take is to develop the abstraction point within your application, and provide one class that consumes libplctag.NET, and another that provides the mocking behaviour. Something like this example.

[sebastian-zitzelsberger]

Thanks for the quick response. I aware of that example. But i don't want to write tests against my own abstraction. I want to follow the approach proposed here with extension methods on the Tag class for converting the results from the lib to my desired data-types. That's why i replied on the other issue. I want to use real instances of the Tag, but mock any interaction with the native-lib (INative ). Then i can test for any type of byte formats, or exceptions from the device in my tests without having a need for real devices but i would still be using the api of the Tag class. Therefore i would need the INative interface to be public as well as the Constructor in the Tag class that can be configured with an instance of it. I will try to provide a PR to demonstrate.

[sebastian-zitzelsberger]

I added PR-451 to demonstrate what i would like to do. It also adds some additional getters in the tag, in order to setup my mocks correctly.
I was too lazy for now to manually create any byte array for my test. So i just used the marshaling of my struct, which makes the test rather redundant but it should hopefully get my point across. With these changes, i can mock any byte[] structure for the buffer without any devices. I could also create a static INative Stub that returns different results based on the paths if i don't want to use Mocking-frameworks. So i could easily write different Stubs for different device-types etc.

[timyhac]

I've taken a quick look at the PR.

My first thought is that you would be able to achieve better testability if the extension parameter was byte[] rather than Tag. Within the extension method, the only call to Tag itself is Tag.GetBuffer() - I don't understand the value in testing that.
Another benefit in accepting byte[] is that you could also use it for one of the GetBuffer overloads that accepts an existing array - avoiding the overhead of many short-lived little buffers.

I'm not sure what the purpose of exposing Tag.NativeTagHandle is - the only reference is a test assertion.

I'm definitely missing something - you refer to writing code for different device types but I don't see this in the PR.

Site note - if you don't mind, is it possible to avoid format changes in the PR - it adds a lot of noise to the diff.

[timyhac]

Apologies if I come across as dismissive - I really do appreciate the effort you've gone to to produce this PR, it makes the conversation much more productive! I should have some more time over the next few days to take a more thorough look at this work.

[sebastian-zitzelsberger]

I've taken a quick look at the PR.

My first thought is that you would be able to achieve better testability if the extension parameter was byte[] rather than Tag. Within the extension method, the only call to Tag itself is Tag.GetBuffer() - I don't understand the value in testing that. Another benefit in accepting byte[] is that you could also use it for one of the GetBuffer overloads that accepts an existing array - avoiding the overhead of many short-lived little buffers.

You are right. For testing just the marshaling, i don't need the tag and can just write unit-tests on byte arrays.
However, that was not my (only) goal - even so that's mostly the stuff that is tested in this one unit-test. Sorry, i was a bit lazy and did not provide more tests - but i am also new to the PLC-domain and especially the protocols used by rockwell, so i don't really know very much yet about the concrete applications. I am just trying to apply my general software-engineering knowledge plus the little that i know already about how the protocols and the library work.

I'm not sure what the purpose of exposing Tag.NativeTagHandle is - the only reference is a test assertion.

The Tag class itself is more than just a delegator for the calls to the native c-lib. it also has quite a lot of additional (mostly technical) code that i want to test in integration with my application. If I want to do this at the moment i need to either have real devices that produce the replies or mock the responses to the network calls made by the c-lib.

When i have access to the INative interface and can configure it how i like in my integration test and have the option to define the results in any way i like on a much higher abstraction level in a higher programming language. I will try to provide more details in the pr to hopefully make my point more clear.

I'm definitely missing something - you refer to writing code for different device types but I don't see this in the PR.

I will try to add more to the pr - but keep in mind i am new to the domain, so don't pin me on any details that don't make sense.

Site note - if you don't mind, is it possible to avoid format changes in the PR - it adds a lot of noise to the diff.

Sure, will undo those. I am normally a java-dev and just returned to the net world after a pause of a few years - so my .net idea is not configured correctly yet and applied an auto-format. on that note, is there a code-style scheme for the repo that i can use?

[sebastian-zitzelsberger]

Apologies if I come across as dismissive - I really do appreciate the effort you've gone to to produce this PR, it makes the conversation much more productive! I should have some more time over the next few days to take a more thorough look at this work.

No worries. I am very thankful for the quick response and the discussion. You are the maintainer of an open-source lib - so you are the hero here.

[timyhac]

There is also the ab-server simulator which would fulfill the function of mocking network calls.
It is primarily a simulator, as opposed to a test fixture (although it is used by the c library test suite), so it is limited in the scenarios that could be tested.
But on the surface this would be the tool to reach for if just trying to avoid real devices during development.

In terms of reasons why I made INative internal:
INative is not an abstraction, it is purely there as a way to test the wrapper in isolation from the core library.
An implementation of INative of anything other than the precise and particular behaviour of the core library isn't supported.
SOLID compliance is not a design goal.
I realize this is selfish, but I'd like to avoid supporting library users that develop custom INative implementations.

I think integration testing would be best done by including the core library in the test.
When using libplctag.NET, you're working with 4 different APIs:

1. libplctag.NET
2. libplctag (Ethernet/IP ≈ CIP over TCP)
3. The device interface (e.g. Omron and ControlLogix encode strings and bool arrays differently)
4. The PLC application interface (i.e. Meaning and function of PLC variables)

Yes there is some logic in the wrapper, but the complexity is dwarfed by libplctag itself - particularly since the Mapper since has been removed.
The c library is also where most of the development change is and where the highest value of testing would be.
If you're looking to do integration testing, including as many of these layers as possible would be the right move.

Assuming that you've designed your system such that the system under test includes Tag, I could imagine some of these benefits in isolating the wrapper code from the core library.

• Keeping tests in process.
  This would be helpful in a CI scenario where the test runner has limited memory or time or can't use network APIs.
• Testing the wrapper itself for quality.
  This would especially make sense if you operated in a regulated environment and had already evaluated libplctag itself, but now wanted behaviour guarantees of the wrapper.
• You could test for platforms where there isn't a libplctag binary published.
  libplctag can be compiled for many platforms - only a subset of these are packaged in libplctag.NativeImport nuget package. I'm not sure how many these other platforms can run .NET though.
• You could use more easily use automated testing tools like Stryker or FsCheck.
  I'm not sure how well these tools work when mixing managed and native code.

In terms of testing different devices and failure scenarios, the ab-server and modbus-server simulators don't offer the ability to inject specific errors as far as I'm aware.
Something which I think is common in the PLC domain is that component interfaces are usually poorly documented, and implementations are even worse.
Perhaps some businesses and industries are better at this than others - my experience has certainly been that component/interface quality comes a distant second to the behaviour of the entire system.
If you do go down this path then you'll want to base the tests on observations from a real device under realistic scenarios.

As I've been writing this I think I can get behind the argument there is a gap in the test tooling for libplctag, particularly in invoking edge cases of device behaviour.
Making INative public could get us some of the way there - I wonder if we could add features to ab_server or develop something entirely new that acts as a test fixture and could be used to comprehensively invoke a more comprehensive set of device behaviours.

I like your idea about mocking the network layer. I'm not sure how to do this.

[sebastian-zitzelsberger]

I actually am struggling at the moment after the changes done in #406 to understand what the level of abstraction you want to achieve in this library and what the API is that i should implement against.
All of the old ways before #406 still is still there but marked as deprecated for a long time. Lot of stuff is duplicated now in the examples but much also contains todos, and the conversation in the ticket suggest various ways how certain goals can be achieved without providing full, clear support for any of those versions.

I would argue if you aim for a pure wrapper on top of the c-library, than the Tag-class itself is already some opinionated extension, and not part of the wrapper. Instead the INative interface would be the api and the wrapper's only purpose is to provide an Implementation on that interface with language interoperability and expose the api of the c-lib as a public c#-interface.

And all the other stuff should go into a separate library on top. Then the users can decide what API they want to bind themselves to. The lower level one, that is basically the same as the c-version but in c# or a higher one in c# with additional features and opinionated ways how to use the low level api.

This would mean for me that the Tag class plus all the code for mappings would belong into another library (similar to what you already offered in issue 406) that provides an opinionated way how to use this interface in c# and provide additional features for memory management, async- communication, data-type-conversions, standard-tag definitions etc.

If you aim for the abstraction level of Tag, i would argue it needs a public interface and maybe also needs to be broken down into smaller parts (introducing models for the config of the tag) and you should provide ways for the user to extend its functionality.

I don't understand why you try to prevent users of your library to extend on top of what you have built. I understood that you don't want to hide the complexity from lack of standardization and data-type mappings and i agree on this. But I don't understand why you are so reluctant on making it extensible by making the interface public and let people extend the implementation. You are in no way obligated to provide any support for problems that arise when users break something in the logic by overwriting some stuff. But you also prevent others from reusing what you already built and extend it or simply use the interfaces for abstraction or mocking-purposes or forcing them to fork your repo to make the required changes in order to reuse what you already achieved in this lib.

In regards to the testing stuff, I generally try to avoid writing integration-tests that also test functionality of libraries. Either i am doing a full end-to-end test, or i am writing integration tests that test the integration with the interface that i am programming against, that hopefully has a much higher level of abstraction and ideally is an in process-dependency and thus less effort to setup.

I did add some additional stuff in the pr, that shows what could be possible to build on top for simulating device-functionality if my proposed changes to the visibility would be done. I don't know yet if this really something that i would proceed further (need more experience in the domain first), but you could built a lot of reusable code that would support testing the integration of my own code with the api for various scenarios without requiring any real devices or relying on hard to setup emulations on lower abstraction levels.

[timyhac]

I actually am struggling at the moment after the changes done in #406 to understand what the level of abstraction you want to achieve in this library and what the API is that i should implement against.

Probably the best explanation of this is the docs. It says "Values are strongly typed" - which is not really a notable feature after #406. Worse, the README says "Values are strongly typed (both Atomic and User Defined Types)" which is no longer true.

... the INative interface would be the api and the wrapper's only purpose is to provide an Implementation on that interface with language interoperability and expose the api of the c-lib as a public c#-interface.

This is the purpose of the libplctag.NativeImport package. There are other consumers of this package that I'm aware of which do things differently to the libplctag.NET primary package: https://github.com/ChrisPulman/ABPlcRx https://github.com/mesta1/libplctag-csharp https://github.com/Corsinvest/cv4ab-api-dotnet
Feel free to use this one instead of the primary package!
Even at this layer there is some level of opinion implemented (minimum .NET version, whether the modern P/Invoke features are used, license, ...) making it unsuitable for some use cases.

All of the old ways before #406 still is still there ... how certain goals can be achieved without providing full, clear support for any of those versions.

I see now why the original post was on that issue. My comments in that post should hopefully explain why I moved the mapping logic out of the library into an example. Let me know in a new ticket if you do need help on the examples.

I don't understand why you are so reluctant on making it extensible by making the interface public and let people extend the implementation. You are in no way obligated to provide any support for problems that arise when users break something in the logic by overwriting some stuff. But you also prevent others from reusing what you already built and extend it or simply use the interfaces for abstraction or mocking-purposes or forcing them to fork your repo to make the required changes in order to reuse what you already achieved in this lib.

In my defense, if I wasn't evaluating proposals before accepting them I wouldn't be doing a very good job as a maintainer 😁
IMO the major benefit of upstreaming changes is offloading the maintenance burden to the maintainer and the community.

And forking is always a good option to have - you take control of your dependencies!

[timyhac]

I'm wondering if un-deprecating ITag, and altering it to be a complete interface for Tag would also suit your needs. This is more a question exploring the problem space, not so much a proposal.

I believe other solutions like Ingear offer this kind of thing, they also offer implementations that inject signals like sawtooth or sine wave instead of reading from the device. This could be helpful where the consuming application is a relatively lightweight system that just displays data read from PLCs and the codebase needs to remain very minimal.

[sebastian-zitzelsberger]

@timyhac Thanks for your explanation, your patience, your work and your time. It is much appreciated. I think I will just follow your suggestion and work with the native-import package and build my own abstraction on top of that, which i can design how i like.

In my opinion it would still benefit the library if the changes to the visibility would be done and allow it to be extended, but i can follow some of your thoughts and don't want to attempt to convince you.

The removal of the deprecation of the ITag seems like it could also work, but as using the native import seems like a workable solution for me with all design decisions in my hand, i will try this approach for now. Therefore from my point of view, i would close my pr and you can close this issue.

[kyle-github]

The API of the core C library is very stable. I have not had a release that broke backwards compatibility in many years, maybe a decade.

What could be done to the core library that would help testing? There is this idea that I was thinking about a few years ago. You can simulate this fairly well just using the existing ab_server program. That is why I have not worked on this.

Note that ab_server could have more features as well. Currently it can return the error "duplicate connection ID" and it can delay responses. What features would make it more useful?

[timyhac]

The dummy tag would have a smaller footprint I assume which would be advantageous both for automated testing, it would also be convenient for simulation purposes because library consumers could avoid standing up the ab_server process.

I was going to suggest being able to inject delays - not realizing that ab_server already does this. Excellent!

The other two testing use cases I was thinking of are:

1. (Deterministic) mocks for specific edge cases - I believe this is what sebastian is looking to achieve.
2. Fuzzing / some kind of randomized failure injection that can assist finding bugs in the consuming application.

I haven't thought about how this could be exposed. Thinking out loud maybe an attribute string for the dummy plc type that determines possible values at failure injection points: delay=[100ms,150ms,200ms]&errors=[PLCTAG_ERR_DUPLICATE,PLCTAG_ERR_NO_MEM] - hmmmm not sure.