Skip to content

[Core] Update pytest fixture scope#45563

Merged
pvaneck merged 1 commit intoAzure:mainfrom
pvaneck:core-experimental-test-fix
Mar 9, 2026
Merged

[Core] Update pytest fixture scope#45563
pvaneck merged 1 commit intoAzure:mainfrom
pvaneck:core-experimental-test-fix

Conversation

@pvaneck
Copy link
Copy Markdown
Member

@pvaneck pvaneck commented Mar 6, 2026

Module level scoping caused the flask server to be started and stopped multiple times, causing flakiness.

Make it session-scoped to only start it once.

Module level scoping caused the flask server to be started and stopped
multiple times, causing flakiness.

Make it session-scoped to only start it once.

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
@pvaneck pvaneck marked this pull request as ready for review March 7, 2026 01:23
Copilot AI review requested due to automatic review settings March 7, 2026 01:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces test flakiness in azure-core-experimental by ensuring the Flask-based core test server is started once per test run (instead of repeatedly per module), and by invoking Flask via the active Python interpreter.

Changes:

  • Switches the port pytest fixture from module scope to session scope to avoid repeated server start/stop cycles.
  • Updates test server startup command to use sys.executable -m flask ... instead of relying on flask being on PATH.
Comments suppressed due to low confidence (1)

sdk/core/azure-core-experimental/tests/conftest.py:69

  • cmd is built as a single string and passed to subprocess.Popen. On Windows with shell=False, this can fail when sys.executable contains spaces (e.g., a Python installed under Program Files) because the executable path won’t be parsed/quoted correctly. Consider passing args as a list (e.g., [sys.executable, "-m", "flask", "run", "-p", str(port)]) and using shell=False on all platforms (you can keep preexec_fn=os.setsid for non-Windows) to avoid quoting issues and make the Linux comment about needing shell=True obsolete.
    cmd = f"{sys.executable} -m flask run -p {port}"
    if os.name == "nt":  # On windows, subprocess creation works without being in the shell
        child_process = subprocess.Popen(cmd, env=dict(os.environ))
    else:
        # On linux, have to set shell=True
        child_process = subprocess.Popen(cmd, shell=True, preexec_fn=os.setsid, env=dict(os.environ))

@pvaneck
Copy link
Copy Markdown
Member Author

pvaneck commented Mar 9, 2026

/check-enforcer override

@pvaneck pvaneck merged commit 2d0a9aa into Azure:main Mar 9, 2026
47 of 48 checks passed
@pvaneck pvaneck deleted the core-experimental-test-fix branch March 9, 2026 21:37
aprilk-ms pushed a commit that referenced this pull request Mar 11, 2026
Module level scoping caused the flask server to be started and stopped
multiple times, causing flakiness.

Make it session-scoped to only start it once.

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
singankit pushed a commit that referenced this pull request Mar 16, 2026
Module level scoping caused the flask server to be started and stopped
multiple times, causing flakiness.

Make it session-scoped to only start it once.

Signed-off-by: Paul Van Eck <paulvaneck@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants