[Trivial] Simplify if-else blocks and more descriptive variable naming #12437

pull jeffrade wants to merge 1 commits into bitcoin:master from jeffrade:test_runner_refactor changing 1 files +32 −32
  1. jeffrade commented at 1:49 AM on February 15, 2018: contributor

    Was looking through test_runner.py to start work on #11964. Made a few changes to make the file more readable and keep these separate from future PR.

  2. fanquake added the label Tests on Feb 15, 2018
  3. randolf approved
  4. in test/functional/test_runner.py:427 in 8583e03e58 outdated 
     424 | @@ -425,10 +425,9 @@ def get_next(self):
 425 |              # Return first proc that finishes
 426 |              time.sleep(.5)
 427 |              for j in self.jobs:
    promag commented at 3:10 PM on February 15, 2018:

    Might as well improve others, like s/j/job?

    jeffrade commented at 3:26 PM on February 15, 2018:

    Will do 👍

  5. in test/functional/test_runner.py:55 in 8583e03e58 outdated 
      51 | @@ -52,6 +52,8 @@
  52 |  TEST_EXIT_PASSED = 0
  53 |  TEST_EXIT_SKIPPED = 77
  54 |  
  55 | +TRAVIS_TIMEOUT_DURATION = 1200 # 20 minutes represented in seconds
    promag commented at 3:11 PM on February 15, 2018:

    Keep 20 * 60 expression?

    jeffrade commented at 3:24 PM on February 15, 2018:

    @promag I'm fine either way. Wasn't sure if people would read it as twenty * sixty seconds or not. I can make that change later today unless anyone prefers 1200. Thanks!

  6. jeffrade force-pushed on Feb 16, 2018
  7. jeffrade commented at 1:40 AM on February 16, 2018: contributor

    @promag Additional requests completed. Made additional variable name changes and kept 20 minute timeout as 20 * 60.

  8. jonasschnelli commented at 10:42 AM on February 17, 2018: contributor

    Why not. utACK 67592cb7c43e6cf837a4abef4222bf6d0a1bc607

  9. fanquake requested review from jnewbery on Mar 14, 2018
  10. fanquake requested review from MarcoFalke on Mar 14, 2018
  11. promag commented at 5:04 PM on March 16, 2018: member

    utACK 67592cb.

  12. [Trivial] Simplify if-else blocks and more descriptive variable naming 97bcd36811
  13. in test/functional/test_runner.py:492 in 67592cb7c4 outdated 
     488 | @@ -489,7 +489,7 @@ def check_script_list(src_dir):
 489 |      Check that there are no scripts in the functional tests directory which are
 490 |      not being run by pull-tester.py."""
 491 |      script_dir = src_dir + '/test/functional/'
 492 | -    python_files = set([t for t in os.listdir(script_dir) if t[-3:] == ".py"])
 493 | +    python_files = set([file for file in os.listdir(script_dir) if file[-3:] == ".py"])
    mcdallas commented at 5:30 PM on March 16, 2018:

    file.endswith(".py") is slightly more readable

    jeffrade commented at 6:53 PM on March 16, 2018:

    @mcdallas Agreed and updated. Thanks!

  14. jeffrade force-pushed on Mar 16, 2018
  15. jeffrade commented at 11:22 PM on March 16, 2018: contributor

    Travis job error appears to be The job exceeded the maximum time limit for jobs, and has been terminated.

  16. promag commented at 12:06 AM on March 17, 2018: member

    Restarted failed job.

  17. promag commented at 9:46 AM on March 17, 2018: member

    utACK 97bcd36.

  18. MarcoFalke merged this on Mar 17, 2018
  19. MarcoFalke closed this on Mar 17, 2018
  20. MarcoFalke referenced this in commit 585db41e9a on Mar 17, 2018
  21. MarcoFalke commented at 3:09 PM on March 17, 2018: member

    utACK 97bcd36811b5f3cd1ff8ede379fe2744ef456c2a

  22. MarcoFalke commented at 3:26 PM on March 17, 2018: member

    As an aside note: Since this also changes code, it shouldn't be marked with "trivial".

  23. jeffrade commented at 3:48 PM on March 17, 2018: contributor

    I must have misread the contributing docs.

  24. in test/functional/test_runner.py:254 in 97bcd36811 
     256 | +        test_list = ALL_SCRIPTS
 257 |      else:
 258 | -        # No individual tests have been specified.
 259 | -        # Run all base tests, and optionally run extended tests.
 260 | +        # Run base tests only
 261 |          test_list = BASE_SCRIPTS
    MarcoFalke commented at 5:54 PM on March 17, 2018:

    Could you change this to test_list += BASE_SCRIPTS, otherwise later modification of test_list will also modify the const BASE_SCRIPTS.

    jeffrade commented at 6:51 PM on March 17, 2018:

    I missed that, thank you. PR opened

  25. in test/functional/test_runner.py:251 in 97bcd36811 
     251 |              else:
 252 | -                print("{}WARNING!{} Test '{}' not found in full test list.".format(BOLD[1], BOLD[0], t))
 253 | +                print("{}WARNING!{} Test '{}' not found in full test list.".format(BOLD[1], BOLD[0], test))
 254 | +    elif args.extended:
 255 | +        # Include extended tests
 256 | +        test_list = ALL_SCRIPTS
    MarcoFalke commented at 5:54 PM on March 17, 2018:

    Same here

  26. MarcoFalke referenced this in commit 00d1680498 on Mar 17, 2018
  27. in test/functional/test_runner.py:436 in 97bcd36811 
     437 | +                    # In travis, timeout individual tests (to stop tests hanging and not providing useful output).
 438 |                      proc.send_signal(signal.SIGINT)
 439 |                  if proc.poll() is not None:
 440 |                      log_out.seek(0), log_err.seek(0)
 441 | -                    [stdout, stderr] = [l.read().decode('utf-8') for l in (log_out, log_err)]
 442 | +                    [stdout, stderr] = [file.read().decode('utf-8') for file in (log_out, log_err)]
    jnewbery commented at 9:38 PM on March 18, 2018:

    file() was a builtin function until python 3 (https://docs.python.org/2/library/functions.html#file). Although it's no longer a builtin, my preference would be to avoid using it as a variable name.

    Same for uses of file further down.

    jeffrade commented at 9:52 PM on March 18, 2018:

    @jnewbery Will open a new PR shortly and rename from file to log_file, test_file, etc. where applicable.

  28. jnewbery commented at 9:39 PM on March 18, 2018: member

    post-merge utACK 97bcd36811b5f3cd1ff8ede379fe2744ef456c2a with one nit.

  29. MarcoFalke referenced this in commit 872c921c0a on Mar 19, 2018
  30. PastaPastaPasta referenced this in commit bccff13841 on Jun 14, 2020
  31. PastaPastaPasta referenced this in commit ee27beff3f on Jun 17, 2020
  32. UdjinM6 referenced this in commit a95640cb11 on Jun 17, 2020
  33. PastaPastaPasta referenced this in commit f37566a9f3 on Jun 17, 2020
  34. PastaPastaPasta referenced this in commit 20e64479d7 on Jun 17, 2020
  35. PastaPastaPasta referenced this in commit bcc1054700 on Jun 17, 2020
  36. PastaPastaPasta referenced this in commit 605debf096 on Jun 17, 2020
  37. PastaPastaPasta referenced this in commit e1a949c7b7 on Jun 17, 2020
  38. PastaPastaPasta referenced this in commit 19e2de1b22 on Jun 17, 2020
  39. PastaPastaPasta referenced this in commit 3d252a72b2 on Jun 17, 2020
  40. jasonbcox referenced this in commit 334c2a6c1e on Nov 2, 2020
  41. PastaPastaPasta referenced this in commit 96404b66f0 on Dec 12, 2020
  42. PastaPastaPasta referenced this in commit 40a9646b7c on Dec 12, 2020
  43. PastaPastaPasta referenced this in commit 4af9d68ac6 on Dec 15, 2020
  44. PastaPastaPasta referenced this in commit 9bca3a547a on Dec 15, 2020
  45. PastaPastaPasta referenced this in commit 3cfe3f6db1 on Dec 15, 2020
  46. PastaPastaPasta referenced this in commit 0c76284fc8 on Dec 18, 2020
  47. MarcoFalke locked this on Sep 8, 2021
Contributors
jeffraderandolfpromagjonasschnellimcdallasMarcoFalkejnewbery
Labels
Tests
Linked (view graph)
#11964 Avoid "test_runner.py" script array#12710 Append scripts to new test_list array to fix bad assignment#12720 qa: Avoiding 'file' function name from python2
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-17 09:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me