Skip to content

fix(test): address PR #294 review — targeted teardown and stdout.close in finally #295

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.

Sign up for GitHub

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

Merged
danielmeppiel merged 1 commit into from
Mar 14, 2026
Merged

fix(test): address PR #294 review — targeted teardown and stdout.close in finally #295

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 22 additions & 10 deletions tests/integration/test_auto_install_e2e.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@
"""

import os
import platform
import pytest
import subprocess
import tempfile
import shutil
import time
from pathlib import Path


Expand Down Expand Up @@ -69,10 +71,17 @@ def teardown_method(self):
"""Clean up test environment."""
os.chdir(self.original_dir)
if os.path.exists(self.test_dir):
# ignore_errors=True: on Windows, recently-terminated subprocesses
# may still hold file locks on the temp directory (WinError 32).
# CI temp dirs are ephemeral — safe to leave behind if needed.
shutil.rmtree(self.test_dir, ignore_errors=True)
try:
shutil.rmtree(self.test_dir)
except PermissionError:
if platform.system() == "Windows":
# On Windows, recently-terminated subprocesses may still
# hold file locks (WinError 32). Retry once after a brief
# delay; CI temp dirs are ephemeral so a leftover is OK.
time.sleep(1)
shutil.rmtree(self.test_dir, ignore_errors=True)
else:
raise

def test_auto_install_virtual_prompt_first_run(self, temp_e2e_home):
"""Test auto-install on first run with virtual package reference.
Expand Down Expand Up @@ -129,14 +138,14 @@ def test_auto_install_virtual_prompt_first_run(self, temp_e2e_home):
break

# Wait for graceful shutdown
process.stdout.close()
try:
process.wait(timeout=10)
except subprocess.TimeoutExpired:
process.kill()
process.wait()

finally:
process.stdout.close()
output = ''.join(output_lines)

# Check output for auto-install messages
Expand Down Expand Up @@ -189,11 +198,12 @@ def test_auto_install_uses_cache_on_second_run(self, temp_e2e_home):
if "Package installed and ready to run" in line:
process.terminate()
break
process.stdout.close()
process.wait(timeout=10)
except:
process.kill()
process.wait()
finally:
process.stdout.close()

# Verify package exists
package_path = Path("apm_modules") / "github" / "awesome-copilot" / "skills" / "architecture-blueprint-generator"
Expand Down Expand Up @@ -223,12 +233,12 @@ def test_auto_install_uses_cache_on_second_run(self, temp_e2e_home):
if "Executing" in line or "Package installed and ready to run" in line:
process.terminate()
break
process.stdout.close()
process.wait(timeout=10)
except:
process.kill()
process.wait()
finally:
process.stdout.close()
output = ''.join(output_lines)

# Check output - should NOT show install/download messages
Expand Down Expand Up @@ -271,11 +281,12 @@ def test_simple_name_works_after_install(self, temp_e2e_home):
if "Package installed and ready to run" in line:
process.terminate()
break
process.stdout.close()
process.wait(timeout=10)
except:
process.kill()
process.wait()
finally:
process.stdout.close()

# Run with simple name - early termination
process = subprocess.Popen(
Expand All @@ -301,12 +312,12 @@ def test_simple_name_works_after_install(self, temp_e2e_home):
if "Executing" in line or "Auto-discovered" in line:
process.terminate()
break
process.stdout.close()
process.wait(timeout=10)
except:
process.kill()
process.wait()
finally:
process.stdout.close()
output = ''.join(output_lines)

# Check output - should discover the installed prompt
Expand Down Expand Up @@ -348,11 +359,12 @@ def test_auto_install_with_qualified_path(self, temp_e2e_home):
if "Package installed and ready to run" in line:
process.terminate()
break
process.stdout.close()
process.wait(timeout=10)
except:
process.kill()
process.wait()
finally:
process.stdout.close()

# Check that package was installed
package_path = Path("apm_modules/github/awesome-copilot/skills/architecture-blueprint-generator")
Expand Down
Loading