Skip to content
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

Jump to bottom

fix: pool issues #3266

Merged
merged 66 commits into from
Jul 12, 2024
Merged

fix: pool issues #3266

merged 66 commits into from
Jul 12, 2024

Conversation

germa89
Copy link
Collaborator

@germa89 germa89 commented Jul 10, 2024 •
edited
Loading

Description

Unfortunately this PR has grown too big, and I'm not happy about it.

Several things are made here:

  • Disabled flaky tests. See Flaky tests #2435 (comment)
  • Added ready and wait_for_ready method to MapdlPool.
  • Fix some tests by:
    • Making sure the pool is ready before testing using pool fixture.
    • Making sure each instance is ready before going ahead (see initialized function)
    • Limitate the valid ports after each test (See VALID_PORT variable).
    • Everything is in place to kill the extra instances, or raise an exception. So far, do nothing. See run_before_and_after_tests_3.
    • Fix convert pipe on Windows powershell.
    • Fix behaviour of is_valid_process.
  • Added one test

Minors

  • Fix using the wrong version when using tag latest-**
  • Moved one test from test_mapdl to test_console.
  • A bit of refactor.

Issue linked

Fix pool issues detected in latest release attemp.

Checklist

germa89 and others added 18 commits July 8, 2024 19:00
@germa89
feat: adding port to exception message
53ef4e6
@germa89
fix: attempt tests before being ready
c38b6de
@germa89
feat: changing arguments order
f4ec8c0
@germa89
Merge branch 'main' into fix/tests
699d83d
@germa89
chore: wait for complete exit
325a67f
@germa89
test: raise exception if mapdl instances are alive between tests
dc0f3ed
@pyansys-ci-bot
chore: adding changelog file 3257.added.md
a5f36f5
@germa89
fix: adding missing import
bd50f30
@germa89
fix: adding missing object
f4d36f6
@germa89
test: check process status
3c39186
@germa89
fix: running process check only on local.
f474c23
@germa89
feat: enforcing having exactly the amount of instances specified.
9f207d8 
Adding timeout to check if the instance is already launched.
@germa89
Merge branch 'main' into feat/checking-no-instances-are-left-after-ea…
8a7ca1d 
…ch-test
@germa89
feat: adding a timeout before moving on
a624ba7
@germa89
Merge branch 'main' into fix/pool-issues
2927617
@germa89
fix: latest_version env var
ff6a664
@germa89
feat: added pool_creator fixture. We use pool fixture to check pool h…
9c6e5c3 
…ealth.

fix: some tests
@germa89
fix: NoSuchProcess error. Small cosmetic fix
bd70a43
@germa89 germa89 requested a review from a team as a code owner July 10, 2024 14:47
@germa89 germa89 requested review from clatapie and pyansys-ci-bot and removed request for a team July 10, 2024 14:47
@ansys-reviewer-bot
Copy link
Contributor

Thanks for opening a Pull Request. If you want to perform a review write a comment saying:

@ansys-reviewer-bot review

@germa89 germa89 self-assigned this Jul 10, 2024
@github-actions github-actions bot added CI/CD Related with CICD, Github Actions, etc maintenance General maintenance of the repo (libraries, cicd, etc) bug Issue, problem or error in PyMAPDL labels Jul 10, 2024
@germa89 germa89 changed the title Fix: pool issues fix: pool issues Jul 10, 2024
@github-actions github-actions bot added documentation Documentation related (improving, adding, etc) dependencies labels Jul 12, 2024
@germa89 @pyansys-ci-bot
build: update ansys-api-mapdl to 0.5.2 (#3255)
f06db96 
* build: update ansys-api-mapdl to 0.5.2

* chore: adding changelog file 3255.dependencies.md

---------

Co-authored-by: pyansys-ci-bot <[email protected]>
@github-actions github-actions bot removed documentation Documentation related (improving, adding, etc) dependencies labels Jul 12, 2024
This was referenced Jul 12, 2024
Closed
Merged
germa89 and others added 2 commits July 12, 2024 13:44
@germa89
test: skip flaky test. See #2435 comment
d99d916
@germa89 @pyansys-ci-bot
fix: not showing instances on linux (#3263)
bc2d257 
* fix: not showing instances on linux

* chore: adding changelog file 3263.fixed.md

---------

Co-authored-by: pyansys-ci-bot <[email protected]>
This was referenced Jul 12, 2024
Closed
Open
@germa89 germa89 requested a review from koubaa July 12, 2024 13:30
@germa89 germa89 marked this pull request as ready for review July 12, 2024 13:30
@germa89
Copy link
Collaborator Author

germa89 commented Jul 12, 2024

Pinging @koubaa and @clatapie for review.

clatapie
clatapie reviewed Jul 12, 2024
View reviewed changes
doc/changelog.d/3263.fixed.md Outdated Show resolved Hide resolved
tests/test_pool.py Show resolved Hide resolved
clatapie
clatapie approved these changes Jul 12, 2024
View reviewed changes
Copy link
Contributor

@clatapie clatapie left a comment

Choose a reason for hiding this comment

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

I added 2 small comments but except from this, it looks good to me!

@germa89 germa89 enabled auto-merge (squash) July 12, 2024 15:57
@germa89 germa89 merged commit 2df6130 into main Jul 12, 2024
58 of 61 checks passed
@germa89 germa89 deleted the fix/pool-issues branch July 12, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clatapie clatapie clatapie approved these changes

@RobPasMue RobPasMue RobPasMue left review comments

@pyansys-ci-bot pyansys-ci-bot Awaiting requested review from pyansys-ci-bot pyansys-ci-bot is a code owner automatically assigned from ansys/pymapdl-maintainers

@koubaa koubaa Awaiting requested review from koubaa

Assignees

@germa89 germa89

Labels
bug Issue, problem or error in PyMAPDL CI/CD Related with CICD, Github Actions, etc maintenance General maintenance of the repo (libraries, cicd, etc)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@germa89 @RobPasMue @clatapie @pyansys-ci-bot