Skip to content

fix(cli): check access process on windows#4293

Merged
germa89 merged 10 commits intomainfrom
fix/check-access-process-on-windows
Nov 27, 2025
Merged

fix(cli): check access process on windows#4293
germa89 merged 10 commits intomainfrom
fix/check-access-process-on-windows

Conversation

@SMoraisAnsys
Copy link
Contributor

@SMoraisAnsys SMoraisAnsys commented Nov 6, 2025

Description

When running on windows, the process username() can contain the DOMAIN which makes the comparison fail.

Issue linked

None

Checklist

When running on windows, the process `username()` can contain the DOMAIN which makes the comparison fail.
Copilot AI review requested due to automatic review settings November 6, 2025 14:31
@SMoraisAnsys SMoraisAnsys requested a review from a team as a code owner November 6, 2025 14:31
@SMoraisAnsys SMoraisAnsys self-assigned this Nov 6, 2025
Copy link
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 fixes a Windows-specific bug in the process access check where username comparison fails when the process username includes a domain prefix (e.g., "DOMAIN\username"). The fix extracts the username portion after the backslash on Windows systems.

Key changes:

  • Added platform-specific handling for Windows usernames in _can_access_process()
  • Extracts username from "DOMAIN\username" format by splitting on backslash

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions github-actions bot added the bug Issue, problem or error in PyMAPDL label Nov 6, 2025
Copy link
Collaborator

@germa89 germa89 left a comment

Choose a reason for hiding this comment

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

I forgot to mention, you should add a test for this please :)

@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 91.34%. Comparing base (be80897) to head (9a30857).
⚠️ Report is 3 commits behind head on main.

❌ Your patch status has failed because the patch coverage (66.66%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4293      +/-   ##
==========================================
+ Coverage   91.28%   91.34%   +0.06%     
==========================================
  Files         193      193              
  Lines       15742    15745       +3     
==========================================
+ Hits        14370    14383      +13     
+ Misses       1372     1362      -10     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@SMoraisAnsys
Copy link
Contributor Author

@germa89 The current implementation of the tests do not allow me to test without having mapdl installed (or that's my understand from the need to provide a path for ansysxxx). Would you like me to update the current test so that it patches a few pymapdl commands and runs without requiring any local install ?

Copy link
Collaborator

@germa89 germa89 left a comment

Choose a reason for hiding this comment

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

Thank you @SMoraisAnsys for this PR. It does fix one our current issues that went unnoticed.

Also sorry for the delay to get to this.

LGTM.

Close #4321

@germa89 germa89 enabled auto-merge (squash) November 27, 2025 09:58
@germa89 germa89 merged commit d3c0c5d into main Nov 27, 2025
161 of 167 checks passed
@germa89 germa89 deleted the fix/check-access-process-on-windows branch November 27, 2025 11:51
clatapie pushed a commit that referenced this pull request Feb 18, 2026
* fix(cli): check access process on windows

When running on windows, the process `username()` can contain the DOMAIN which makes the comparison fail.

* ci: auto fixes from pre-commit.com hooks.

for more information, see https://pre-commit.ci

* chore: adding changelog file 4293.fixed.md [dependabot-skip]

* fix: add missing import and take into account review

* test: add test with domain in username

* ci: auto fixes from pre-commit.com hooks.

for more information, see https://pre-commit.ci

* test: remove print statement from domain username handling test

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: pyansys-ci-bot <92810346+pyansys-ci-bot@users.noreply.github.com>
Co-authored-by: German <28149841+germa89@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Issue, problem or error in PyMAPDL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants