diff --git a/.github/workflows/test_pr.yml b/.github/workflows/test_pr.yml index 38cb4e399..13aeb3779 100644 --- a/.github/workflows/test_pr.yml +++ b/.github/workflows/test_pr.yml @@ -30,7 +30,7 @@ jobs: success: name: PR Test Completed - needs: [is_source_changed, test] + needs: [is_source_changed, test, test_macos] runs-on: ubuntu-22.04 if: always() && needs.is_source_changed.result == 'success' && (needs.is_source_changed.outputs.is_source_changed == 'false' || needs.Test.result == 'success') steps: @@ -164,3 +164,36 @@ jobs: run: uv pip install --system .${{ matrix.package-extras }} --group test -c constraints.txt - name: Test Python run: doit test + + test_macos: + name: Test (macOS) + needs: [lint_python] + runs-on: macos-latest + env: + CI: "" + strategy: + matrix: + python-version: ['3.10', '3.13'] + fail-fast: false + + steps: + - name: Checkout repository + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - name: Set up Python + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + with: + python-version: ${{ matrix.python-version }} + - name: Set up uv & Python + uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7.6.0 + with: + enable-cache: true + cache-dependency-glob: "**/pyproject.toml" + python-version: ${{ matrix.python-version }} + - name: Install Python dependencies + run: uv pip install --system ".[all]" --group test -c constraints.txt + # Unit tests only for now. The functional suite needs Zookeeper/Kafka and + # the built UI bundle, and parts of it additionally depend on the host + # resolving its own FQDN, which is not reliable on macOS. Widening this + # to `doit test` is follow-up work. + - name: Test Python + run: pytest tests/unit --verbose diff --git a/dco/brianegge.rst b/dco/brianegge.rst new file mode 100644 index 000000000..42c82bafe --- /dev/null +++ b/dco/brianegge.rst @@ -0,0 +1,81 @@ +DEVELOPER CERTIFICATE OF ORIGIN +=============================== + +Thank you for your interest in the open source software project(s) (the “Project”) maintained or managed by +Morgan Stanley Services Group Inc. (“us” or “we”). The purpose of this Developer Certificate of Origin (the “DCO” or +“Agreement”) is to define the intellectual property license granted by persons or entities that make Contributions +(defined below) to the Project. You, **Brian Egge**, agree and certify as set forth in this Agreement. You may be +contacted at **brianegge@gmail.com**. + +By submitting a Contribution, including but not limited to by pull request, you agree that you have read and +understood this Agreement and you will be legally bound thereby. In consideration of the opportunity to +participate in the community of Project contributors, you hereby agree to the following terms and conditions in +connection with your present and future Contributions: + +1. **Contributions:** + + * The term “Contribution” means any source code, object code, patch, tool, sample, graphic, specification, manual, + documentation, or any other material submitted by you to the Project. + * A Contribution is “submitted” when any form of electronic, verbal, or written communication is sent to the Project, + including but not limited to communication on electronic mailing lists, source code control systems,and issue + tracking systems that are managed by, or on behalf of, the Project for the purpose of discussing or improving + software or documentation of the Project (“Communication”). + * Each Communication that is conspicuously marked or designated by you in writing as “Not a Contribution” will + not be considered a Contribution. + * Any Contribution submitted by you to the Project will be under the terms and conditions of this Agreement + without any additional terms or conditions. + +2. **Grant of License:** You hereby grant us and our affiliates, for purposes of the Project, and to recipients of +software distributed by the Project: + + * a perpetual, irrevocable, non-exclusive, worldwide, fully paid-up, royalty-free, unrestricted license to + exercise all rights (including sublicensing and commercial exploitation) under all worldwide copyrights, + copyright applications and registrations in the Contribution; and + * a perpetual, irrevocable, non-exclusive, worldwide, full paid-up, royalty-free patent license to make, have + made, use, offer to sell, sell, import, and otherwise transfer your Contribution and derivative works thereof, + where such license applies only to those patent claims licensable by you or your affiliates that are necessarily + infringed by your Contribution alone or by combination of your Contribution with the Project to which you + submitted the Contribution. + +3. **Ownership:** + + * Except as set out above, you keep all right, title and interest in your Contribution. + * You represent that: + + * you are the owner of the Contribution or are otherwise legally entitled to grant the above licenses; + * if any third party, including your employer and/or its affiliates, has rights to any intellectual property + included in your Contribution, then (i) each such third party has provided you written permission to make the + Contribution as specified herein or a written waiver of such rights in and to your + Contribution, **[and (ii) such third party(ies) is/are ]**; + * your Contribution is an original work created by you, and except for third parties who have given permission + to make the Contribution as set forth above, to your knowledge, no other person or entity has claimed, + claims, or has the right to claim any right whatsoever in the Contribution; and + * your Contribution includes complete details of any third party license(s) or other restriction(s) + (including, but not limited to, related copyrights, patents and trademarks) of which you are aware and + which are associated with any part of your Contribution, and of all matters required to be disclosed under + such third party licenses (such as all applicable copyright, patent, trademark and attribution notices, + and all modifications made to certain open source software). + +4. **Notice; Inclusion; No Confidentiality:** + + * You agree to notify us of any facts or circumstances of which you become aware that would make these + representations inaccurate in any respect. Notices and other communications to be sent as directed in the + applicable Project + * Neither we nor the Project is under any obligation to accept and include your Contribution, or to return it to you. + * You will not, absent a separate written agreement signed by us, impose any confidentiality obligations on us, + and we have not undertaken any obligation to treat any Contributions or other information you have or will + give us as confidential or proprietary information. + * You understand and agree that all Contributions including all personal information you submit with it may be + maintained indefinitely and may be redistributed consistent with the applicable open source license(s). + +5. **Effective Date:** The rights that you grant to us under these terms are effective on the date you first +submit a Contribution to us, even if your submission took place before the date you accept the terms of this Agreement. + +6. **Governing Law; Entire Agreement:** This Agreement is governed by the laws of the State of New York, +without regard to its choice of law provisions, and by the laws of the United States. This Agreement sets +forth the entire understanding and agreement between the parties, and supersedes any previous communications, +representations or agreements, whether oral or written, regarding the subject matter herein. No alteration, waiver, +amendment, change or supplement hereto shall be binding or effective unless the same is set forth in writing +signed by both parties. We may freely assign our rights or obligations under this Agreement. + + diff --git a/doc/newsfragments/3952_changed.macos_psutil_support.rst b/doc/newsfragments/3952_changed.macos_psutil_support.rst new file mode 100644 index 000000000..14c7cffec --- /dev/null +++ b/doc/newsfragments/3952_changed.macos_psutil_support.rst @@ -0,0 +1,4 @@ +Fix two macOS-only ``psutil`` failures and run the unit suite on a macOS CI runner. +``TestRunner._is_remote_process_alive`` no longer propagates ``psutil.AccessDenied`` when it +cannot enumerate TCP connections, and the resource monitor only requests ``io_counters`` on +platforms where ``psutil`` implements it. diff --git a/doc/newsfragments/3954_changed.driver_timing_margin.rst b/doc/newsfragments/3954_changed.driver_timing_margin.rst new file mode 100644 index 000000000..cc8b8f1b8 --- /dev/null +++ b/doc/newsfragments/3954_changed.driver_timing_margin.rst @@ -0,0 +1,2 @@ +Widen the upper bounds in ``TestDriverTiming::test_driver_timings``. The assertions left only +100ms of headroom over the driver's own sleeps, which a slow CI host can exceed. diff --git a/testplan/monitor/resource.py b/testplan/monitor/resource.py index f17496492..9a268f336 100755 --- a/testplan/monitor/resource.py +++ b/testplan/monitor/resource.py @@ -23,6 +23,21 @@ from testplan.runners.pools import communication from testplan.common.serialization.base import serialize, deserialize +# psutil does not implement Process.io_counters on every platform - notably it +# is absent on macOS. Asking as_dict() for an attribute psutil does not define +# raises ValueError, so only request it where it actually exists; the per- +# process IO metrics are then simply reported as zero. +_HAS_PROC_IO_COUNTERS = hasattr(psutil.Process, "io_counters") + +_PROC_ATTRS = [ + "pid", + "name", + "memory_info", + "cpu_percent", + "cmdline", + "create_time", +] + (["io_counters"] if _HAS_PROC_IO_COUNTERS else []) + @dataclasses.dataclass class ResourceData: @@ -250,17 +265,7 @@ def collect_process_data(self) -> None: self.last_process_resource = {} for proc in processes: try: - raw_data = proc.as_dict( - attrs=[ - "pid", - "name", - "memory_info", - "cpu_percent", - "cmdline", - "io_counters", - "create_time", - ] - ) + raw_data = proc.as_dict(attrs=_PROC_ATTRS) except psutil.NoSuchProcess: continue cpu_percent = float(raw_data["cpu_percent"]) diff --git a/testplan/runnable/base.py b/testplan/runnable/base.py index 8f898e900..eefad2e3c 100644 --- a/testplan/runnable/base.py +++ b/testplan/runnable/base.py @@ -1405,7 +1405,23 @@ def _is_remote_process_alive(self, host: str, port: int) -> bool: except ValueError: return False - for conn in psutil.net_connections(kind="tcp"): + try: + connections = psutil.net_connections(kind="tcp") + except psutil.AccessDenied: + # Enumerating system-wide sockets is privileged on some platforms + # (notably macOS, where it requires root). We cannot confirm the + # remote instance either way, so fall through to the local PID + # check rather than blocking the run outright. + self.logger.debug( + "Cannot enumerate TCP connections to check whether a remote " + "testplan instance on %s:%s is still alive - insufficient " + "privileges. Falling back to the local PID check.", + host, + port, + ) + return False + + for conn in connections: if ( conn.status == psutil.CONN_ESTABLISHED and conn.raddr diff --git a/tests/unit/testplan/runnable/test_base.py b/tests/unit/testplan/runnable/test_base.py index 0368aecf8..af6ad43a6 100644 --- a/tests/unit/testplan/runnable/test_base.py +++ b/tests/unit/testplan/runnable/test_base.py @@ -1,6 +1,8 @@ -import pytest +import collections import os + import psutil +import pytest from testplan.base import TestplanMock from testplan.common.report import ReportCategories @@ -16,6 +18,20 @@ from testplan.testing.multitest import MultiTest, suite +# Only the fields _is_remote_process_alive() looks at. Declared here rather +# than reusing psutil's own namedtuple, whose location is a private detail +# that has moved between psutil releases. +_Addr = collections.namedtuple("_Addr", ["ip", "port"]) +_Conn = collections.namedtuple("_Conn", ["status", "raddr"]) + + +def _established_conn(host: str, port: int) -> _Conn: + """Build an ESTABLISHED connection record pointing at host:port.""" + return _Conn( + status=psutil.CONN_ESTABLISHED, raddr=_Addr(ip=host, port=port) + ) + + def test_result_for_failed_task(): task = Task( target="dummy_target", @@ -122,7 +138,7 @@ def test_check_pidfile_another_process(self, tmpdir): ): plan._check_pidfile() - def test_check_pidfile_stale_remote_process(self, tmpdir): + def test_check_pidfile_stale_remote_process(self, tmpdir, mocker): """ Test PID file with stale remote process {host}:{port};{pid} format. @@ -134,17 +150,15 @@ def test_check_pidfile_stale_remote_process(self, tmpdir): plan._runpath = str(tmpdir) plan._pidfile_path = os.path.join(plan._runpath, "testplan.pid") - existing_connections = set() - for conn in psutil.net_connections(kind="tcp"): - if conn.status == psutil.CONN_ESTABLISHED and conn.raddr: - existing_connections.add((conn.raddr.ip, conn.raddr.port)) + # Stub out the connection table rather than reading the real one: + # enumerating system-wide sockets needs root on macOS, and on any + # platform the result depends on whatever the host happens to be + # doing at the time. + mocker.patch.object(psutil, "net_connections", return_value=[]) fake_host = "192.0.2.1" fake_port = 65432 - while (fake_host, fake_port) in existing_connections: - fake_port += 1 - fake_pid = 999999 while psutil.pid_exists(fake_pid): fake_pid += 1 @@ -154,7 +168,7 @@ def test_check_pidfile_stale_remote_process(self, tmpdir): plan._check_pidfile() - def test_check_pidfile_active_remote_process(self, tmpdir): + def test_check_pidfile_active_remote_process(self, tmpdir, mocker): """ Test PID file with active remote process {host}:{port};{pid} format. @@ -167,19 +181,14 @@ def test_check_pidfile_active_remote_process(self, tmpdir): plan._runpath = str(tmpdir) plan._pidfile_path = os.path.join(plan._runpath, "testplan.pid") - active_conn = None - for conn in psutil.net_connections(kind="tcp"): - if ( - conn.status == psutil.CONN_ESTABLISHED - and conn.raddr - and conn.raddr.ip - and conn.raddr.port - ): - active_conn = conn - break - - if active_conn is None: - pytest.skip("No active TCP connection found for testing") + # Present a single ESTABLISHED connection to the address recorded in + # the PID file. Using the real connection table would need root on + # macOS and would skip entirely on an otherwise idle host. + active_host, active_port = "192.0.2.1", 65432 + active_conn = _established_conn(active_host, active_port) + mocker.patch.object( + psutil, "net_connections", return_value=[active_conn] + ) fake_pid = 999999 while psutil.pid_exists(fake_pid): diff --git a/tests/unit/testplan/testing/multitest/driver/test_driver.py b/tests/unit/testplan/testing/multitest/driver/test_driver.py index 158d2a2d5..e041426c9 100644 --- a/tests/unit/testplan/testing/multitest/driver/test_driver.py +++ b/tests/unit/testplan/testing/multitest/driver/test_driver.py @@ -195,9 +195,12 @@ def test_driver_timings(self): assert isinstance( driver.timer[ResourceTimings.RESOURCE_SETUP][0], Interval ) - # there is some UnicodeEncodeError when using pytest.approx + # there is some UnicodeEncodeError when using pytest.approx. + # Upper bounds are loose on purpose: this checks that an interval was + # recorded around the sleep, not how fast the runner is. A busy CI + # host can add a couple of hundred milliseconds of scheduling delay. assert ( - 0.1 < driver.timer[ResourceTimings.RESOURCE_SETUP][0].elapsed < 0.3 + 0.1 < driver.timer[ResourceTimings.RESOURCE_SETUP][0].elapsed < 0.6 ) driver.stop() @@ -208,5 +211,5 @@ def test_driver_timings(self): assert ( 0 < driver.timer[ResourceTimings.RESOURCE_TEARDOWN][0].elapsed - < 0.2 + < 0.5 )