-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[confcom] Run testing against built extension #9429
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
Changes from all commits
cae20b3
8131283
73743ec
64a918b
d5ea516
1f57a4a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,77 @@ | ||||||||||||
| # -------------------------------------------------------------------------------------------- | ||||||||||||
| # Copyright (c) Microsoft Corporation. All rights reserved. | ||||||||||||
| # Licensed under the MIT License. See License.txt in the project root for license information. | ||||||||||||
| # -------------------------------------------------------------------------------------------- | ||||||||||||
|
|
||||||||||||
| import fcntl | ||||||||||||
| import importlib | ||||||||||||
| import os | ||||||||||||
| import subprocess | ||||||||||||
| import tempfile | ||||||||||||
| import psutil | ||||||||||||
| import pytest | ||||||||||||
| import sys | ||||||||||||
| import shutil | ||||||||||||
|
|
||||||||||||
| from pathlib import Path | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| # This fixture ensures tests are run against final built wheels of the extension | ||||||||||||
| # instead of the unbuilt local code, which may have breaking differences with | ||||||||||||
| # the thing we actually ship to users. All but the test modules themselves are | ||||||||||||
| # replaced with the wheel in case the tests themselves rely on unshipped code. | ||||||||||||
| @pytest.fixture(autouse=True, scope="session") | ||||||||||||
| def run_on_wheel(request): | ||||||||||||
|
|
||||||||||||
| modules_to_test = {i.module for i in request.session.items} | ||||||||||||
| extensions_to_build = {module.__name__.split(".")[0] for module in modules_to_test} | ||||||||||||
| extension_dirs = {Path(a.split("/azext_")[0]) for a in request.config.args} | ||||||||||||
|
|
||||||||||||
| # Azdev doesn't respect the session scope of the fixture, therefore we need | ||||||||||||
| # to implement equivalent behaviour by getting a unique ID for the current | ||||||||||||
| # run and using that to determine if wheels have already been built. Search | ||||||||||||
| # process parentage until we find the first shell process and use it's | ||||||||||||
| # child's PID as the run ID. | ||||||||||||
| parent = psutil.Process(os.getpid()) | ||||||||||||
| while not any(parent.cmdline()[0].endswith(i) for i in ["bash", "sh"]): | ||||||||||||
| parent = parent.parent() | ||||||||||||
| RUN_ID = parent.children()[0].pid | ||||||||||||
|
|
||||||||||||
| build_dir = Path(tempfile.gettempdir()) / f"wheels_{RUN_ID}" | ||||||||||||
| build_dir.mkdir(exist_ok=True) | ||||||||||||
|
|
||||||||||||
| # Build all extensions being tested into wheels | ||||||||||||
| for extension in extensions_to_build: | ||||||||||||
|
|
||||||||||||
| extension_name = extension.replace("azext_", "") | ||||||||||||
|
|
||||||||||||
| # Ensure we acquire a lock while operating on the build dir to avoid races | ||||||||||||
| lock_file = build_dir / ".dir.lock" | ||||||||||||
| with lock_file.open("w") as f: | ||||||||||||
| fcntl.flock(f, fcntl.LOCK_EX) | ||||||||||||
| try: | ||||||||||||
|
|
||||||||||||
| # Delete the extensions build dir, as azdev extension build doesn't | ||||||||||||
| # reliably handle changes | ||||||||||||
| for extension_dir in extension_dirs: | ||||||||||||
| if (extension_dir / "build").exists(): | ||||||||||||
| shutil.rmtree((extension_dir / "build").as_posix(), ignore_errors=True) | ||||||||||||
|
|
||||||||||||
| if not any(build_dir.glob(f"{extension_name}*.whl")): | ||||||||||||
| subprocess.run( | ||||||||||||
| ["azdev", "extension", "build", extension.replace("azext_", ""), "--dist-dir", build_dir.as_posix()], | ||||||||||||
|
||||||||||||
| ["azdev", "extension", "build", extension.replace("azext_", ""), "--dist-dir", build_dir.as_posix()], | |
| ["azdev", "extension", "build", extension_name, "--dist-dir", build_dir.as_posix()], |
Copilot
AI
Nov 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic on line 71 uses __next__() which will raise StopIteration if no wheel files are found in build_dir. This could happen if the build process fails silently or if the wheel files have unexpected names. Consider adding error handling or validation to ensure at least one wheel was built successfully.
| sys.path.insert(0, build_dir.glob("*.whl").__next__().as_posix()) | |
| wheel_files = list(build_dir.glob("*.whl")) | |
| if not wheel_files: | |
| raise RuntimeError(f"No wheel files found in {build_dir}. The build may have failed or produced unexpected file names.") | |
| sys.path.insert(0, wheel_files[0].as_posix()) |
Copilot
AI
Nov 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable extension is referenced on line 73 outside the loop where it's defined (lines 44-67). This will only work correctly if there's exactly one extension in extensions_to_build. If there are multiple extensions, only modules from the last extension will be reloaded. Consider moving the module reloading logic inside the extension loop or storing all extension names to iterate over later.
| if extension in module.__name__ and module not in modules_to_test: | |
| if any(ext in module.__name__ for ext in extensions_to_build) and module not in modules_to_test: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition on line 36 assumes the parent process command line will end with "bash" or "sh", which may not be true in all environments (e.g., zsh, fish, Windows, or when run from other tools). This could cause an infinite loop or crash if no such parent is found. Consider adding a maximum depth check or handling other shell types.