google / packetdrill

The official Google release of packetdrill
GNU General Public License v2.0
887 stars 220 forks source link

Avoid Python error in case of timeout #40

Closed matttbe closed 4 years ago

matttbe commented 4 years ago

When a test is blocked and after a timeout, this Python error is produced:

  Exception in thread Thread-1:
  Traceback (most recent call last):
    File "/usr/lib/python3.6/threading.py", line 916, in _bootstrap_inner
      self.run()
    File "packetdrill/run_all.py", line 169, in run
      self.testset.RunDir(self.path)
    File "packetdrill/run_all.py", line 147, in RunDir
      self.PollTestSet(procs, time_start)
    File "packetdrill/run_all.py", line 136, in PollTestSet
      for proc, path, variant in procs:
  ValueError: too many values to unpack (expected 3)

That's simply because procs variable is an array of tuple containing 5 items. Two additional items are missing: stdout and stderr.

With the patch, we have a clearer error:

  KILL [/opt/packetdrill/gtests/net/mptcp/mp_join/mp_join_client.pkt (ipv4)]

A second commit prints more info in case of timeout if we are in verbose with log_on_error option.

googlebot commented 4 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

matttbe commented 4 years ago

@googlebot I signed it!

googlebot commented 4 years ago

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

matttbe commented 4 years ago

Code looks great, thanks. Please see the bot report on needing a CLA sign off.

@wdebruij thank you for the quick review! My administration work is now done :)

nealcardwell commented 4 years ago

Merged. Thanks, Matthieu!