Extract init communication

[BenediktBurger]

In order to expand all instrument tests, I moved all the communication in the __init__ methods into their own _init_communication method.
That allows to replace that method for certain tests.

The name of that method could be different, if required.

[BenediktBurger]

I got a new idea:

1. instead of defining a special method just for tests (which is bad practice to write the code for tests).
2. all instruments must have a parameter _init_comm_pairs which contains the init communication pairs for tests (defaults to an empty list)

Reasoning:

• It is a small memory burden to carry a list.
• It does not require additional design considerations for the coding of the __init__
• The same list can be used for the protocol tests

ToDos, if this idea gets accepted:

• write changelog
• add documentation how to treat the instruments

EDIT: The code reflects this idea.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.81%. Comparing base (92175a1) to head (0d44cf1).

❗ Current head 0d44cf1 differs from pull request most recent head 8a0a00e. Consider uploading reports for the commit 8a0a00e to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1005      +/-   ##
==========================================
+ Coverage   58.48%   58.81%   +0.33%     
==========================================
  Files         262      261       -1     
  Lines       18198    18121      -77     
==========================================
+ Hits        10643    10658      +15     
+ Misses       7555     7463      -92     

Flag	Coverage Δ
unittests	58.81% <100.00%> (+0.33%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[BenediktBurger]

What do you think, @bilderbuchi ?

[bilderbuchi]

Thanks for asking for my feedback! :-)

2. all instruments must have a parameter _init_comm_pairs which contains the init communication pairs for tests (defaults to an empty list)

I think test-related code should not leak into the instrument codebase. With this change, now part of the protocol test definition resides in the test files, and part in the instrument implementation (_init_comm_pairs) -- I find this split quite confusing/suboptimal.

I'm not sure why this is necessary in the first place because I could only give this a skim-read? Especially, because in the test files you now extract it back to a module-level constant again, anyway? (e.g. AFG_init_comm = AWG401x_AFG._init_comm_pairs)

Is this pattern the reason? Is this purely for test_all_instruments being able to test instruments that have some communication in __init__?

Can't we inject that _init_comm_pairs attribute in the test code and get an identical effect?
E.g. maintain a Defaultdict(None) of (cls: _init_comm_pairs) items somewhere in tests/ and inject those if available?
cls(adapter=ProtocolAdapter(INITCOMMAP[cls]))

A drawback of this is that the init comm pairs reside not with the test files of the relevant instrument.
Instead, we could require to place those in a tuple with a specified name, and a lookup method tries to find if such a tuple exists in the tests/instruments/manufacturer/instrument module. It's a bit "spooky action at a distance" but pytest is full of this magic, anyway. :-)
cls(adapter=ProtocolAdapter(look_up_initcomm(cls)))

[BenediktBurger]

Is this pattern the reason? Is this purely for test_all_instruments being able to test instruments that have some communication in __init__?

Exactly, the change is purely to be able to test instruments that communicate in their __init__.

Yeah, maybe we can find a way to extract the init_comm_pairs from the test files.