Repository: bitcoin/bitcoin
File: test/functional/test_runner.py (Python)

Code before:
            log_stderr = tempfile.SpooledTemporaryFile(max_size=2**16)
            test_argv = test.split()
            testdir = "{}/{}_{}".format(self.tmpdir, re.sub(".py$", "", test_argv[0]), portseed)
            tmpdir_arg = ["--tmpdir={}".format(testdir)]
            self.jobs.append((test,
                              time.time(),
                              subprocess.Popen([sys.executable, self.tests_dir + test_argv[0]] + test_argv[1:] + self.flags + portseed_arg + tmpdir_arg,
                                               text=True,
                                               stdout=log_stdout,
                                               stderr=log_stderr),
                              testdir,
                              log_stdout,
                              log_stderr))
        if not self.jobs:
            raise IndexError('pop from empty list')

        # Print remaining running jobs when all jobs have been started.
        if not self.test_list:
            print("Remaining jobs: [{}]".format(", ".join(j[0] for j in self.jobs)))

        dot_count = 0
        while True:
            # Return all procs that have finished, if any. Otherwise sleep until there is one.
            time.sleep(.5)
            ret = []
            for job in self.jobs:
                (name, start_time, proc, testdir, log_out, log_err) = job
                if proc.poll() is not None:
                    log_out.seek(0), log_err.seek(0)
                    [stdout, stderr] = [log_file.read().decode('utf-8') for log_file in (log_out, log_err)]
                    log_out.close(), log_err.close()
                    skip_reason = None
                    if proc.returncode == TEST_EXIT_PASSED and stderr == "":
                        status = "Passed"
                    elif proc.returncode == TEST_EXIT_SKIPPED:
                        status = "Skipped"
                        skip_reason = re.search(r"Test Skipped: (.*)", stdout).group(1)
                    else:
                        status = "Failed"
                    self.jobs.remove(job)
                    if self.use_term_control:
                        clearline = '\r' + (' ' * dot_count) + '\r'
                        print(clearline, end='', flush=True)
                    dot_count = 0
                    ret.append((TestResult(name, status, int(time.time() - start_time)), testdir, stdout, stderr, skip_reason))
            if ret:
                return ret
            if self.use_term_control:
                print('.', end='', flush=True)
            dot_count += 1


Review comment (performance):
My understanding is that in Python a list `self.test_list.pop(0)` is O(n) so consuming all elements is quadratic this way.
A `collections.deque` with `popleft` might describe the desired behavior better (performance-wise it would likely be the same for a few hundred elements)

nit: `jobs` isn't a "list" anymore

----

edit: https://docs.python.org/3/tutorial/datastructures.html#using-lists-as-queues
> While appends and pops from the end of list are fast, doing inserts or pops from the beginning of a list is slow (because all of the other elements have to be shifted by one).

Code after:
            log_stderr = tempfile.SpooledTemporaryFile(max_size=2**16)
            test_argv = test.split()
            testdir = "{}/{}_{}".format(self.tmpdir, re.sub(".py$", "", test_argv[0]), portseed)
            tmpdir_arg = ["--tmpdir={}".format(testdir)]

            def proc_wait(task):
                task[2].wait()
                return task

            task = [
                test,
                time.time(),
                subprocess.Popen(
                    [sys.executable, self.tests_dir + test_argv[0]] + test_argv[1:] + self.flags + portseed_arg + tmpdir_arg,
                    text=True,
                    stdout=log_stdout,
                    stderr=log_stderr,
                ),
                testdir,
                log_stdout,
                log_stderr,
            ]
            fut = self.executor.submit(proc_wait, task)
            self.jobs[fut] = test
        if not self.jobs:
            raise IndexError('pop from empty list')

        # Print remaining running jobs when all jobs have been started.
        if not self.test_list:
            print("Remaining jobs: [{}]".format(", ".join(sorted(self.jobs.values()))))

        dot_count = 0
        while True:
            # Return all procs that have finished, if any. Otherwise sleep until there is one.
            procs = futures.wait(self.jobs.keys(), timeout=.5, return_when=futures.FIRST_COMPLETED)
            self.jobs = {fut: self.jobs[fut] for fut in procs.not_done}
            ret = []
            for job in procs.done:
                (name, start_time, proc, testdir, log_out, log_err) = job.result()

                log_out.seek(0), log_err.seek(0)
                [stdout, stderr] = [log_file.read().decode('utf-8') for log_file in (log_out, log_err)]
                log_out.close(), log_err.close()
                skip_reason = None
                if proc.returncode == TEST_EXIT_PASSED and stderr == "":
                    status = "Passed"
                elif proc.returncode == TEST_EXIT_SKIPPED:
                    status = "Skipped"
                    skip_reason = re.search(r"Test Skipped: (.*)", stdout).group(1)
                else:
                    status = "Failed"