Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test suite for Tektronix DPO4104 and bug fixes #253

Merged
merged 3 commits into from
Sep 2, 2020

Conversation

trappitsch
Copy link
Contributor

Full coverage test suite for Tektronix DPO4104
Trying to make use of hypothesis where it makes sense and leave it away where it
doesn't.

Bug fixes for tekdpo4104.py:

  • Instead of the new value to set, the documentation was passed on
    in the _parent_property routine. This routine is only used once,
    however, a usage scenario is written but has likely never been
    tested before. The test that does what this property is supposed to
    do passes now with the fix.

  • Reading ASCII data from a data source used map and then directly
    transferred to an ndarray. This was fine in python 2, not anymore
    though. Fixed on line 114.

  • Reading binary data is tested now. The test however failed since the
    next command issued left only a termination character. The binary data
    is read using binblockread from Instruments. This will leave the
    last termination character behind. Inserted an additional reading
    statement to read one character to get rid of this character. Checked
    with the manual to ensure consistency. Manual states that this
    termination character should be there after the binary data block,
    thus, the bug fix makes sense.
    From the manual on the data type that is expected to be returned:

    <Block> is the waveform data in binary format. The waveform is formatted as:
    #<x><yyy><data><newline>, where:
    <x> is the number of y bytes. For example, if <yyy>=500, then <x>=3)
    <yyy> is the number of bytes to transfer. If width is 1, then all bytes on the
    bus are single data points. If width is 2, then all bytes on the bus are 2-bytes
    wide. Use the WFMInpre:BYT_Nr command to set the width for waveforms
    transferred into the oscilloscope. Use WFMOutpre:BYT_Nr to set the width
    for waveforms transferred out from the oscilloscope.
    <data> is the curve data.
    <newline> is a single byte new line character at the end of the data.

Full coverage test suite for Tektronix DPO4104 added. Trying to make
use of hypothesis where it makes sense and leave it away where it
doesn't.

Bug fixes for `tekdpo4104.py`:
- Instead of the new value to set, the documentation was passed on
  in the `_parent_property` routine. This routine is only used once,
  however, a usage scenario is written but has likely never been
  tested before. The test that does what this property is supposed to
  do passes now with the fix.
- Reading ASCII data from a data source used `map` and then directly
  transferred to an `ndarray`. This was fine in python 2, not anymore
  though. Fixed on line 114.
- Reading binary data is tested now. The test however failed since the
  next command issued left only a termination character. The binary data
  is read using `binblockread` from `Instruments`. This will leave the
  last termination character behind. Inserted an additional reading
  statement to read one character to get rid of this character. Checked
  with the manual to ensure consistency. Manual states that this
  termination character should be there after the binary data block,
  thus, the bug fix makes sense.


@pytest.mark.parametrize("channel", channels_to_try, ids=channels_to_try_ids)
@given(coupling=st.sampled_from(ik.tektronix.TekDPO4104.Coupling))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tried this, but can you pytest parametrize over an enum?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tried it out for fun, in fact you can :) Here would be:

@pytest.mark.parametrize("coupling", ik.tektronix.TekDPO4104.Coupling)

That would probably make more sense than sampling with hypothesis.

@scasagrande
Copy link
Contributor

Changes look good to me

Refreshing the branch will get your coverage results pushed to coveralls.io for nice coverage diffs

@trappitsch
Copy link
Contributor Author

Thanks @scasagrande, I moved the random sampling over the enum over to parametrizing it with pytest. Pytest is great :) Also refreshed the branch, thanks for fixing the coverage stuff.

@scasagrande
Copy link
Contributor

Awesome, looks great to me

@scasagrande
Copy link
Contributor

We'll just see what coveralls says when its done parsing our results. Take a look when you can, and if we're good I'll merge when I get back to my desk tonight

@trappitsch
Copy link
Contributor Author

Looking good, 100.0 on tekdpo4104.py. Seems to be working great! On to the next instrument...

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 91.137% when pulling 73dc4d7 on trappitsch:tests_tekdpo4104 into cce7b72 on Galvant:master.

@scasagrande scasagrande merged commit d5c1ad5 into instrumentkit:master Sep 2, 2020
@trappitsch trappitsch deleted the tests_tekdpo4104 branch September 2, 2020 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants