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.

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

refactor: small improvements to test settings #3577

Open
wants to merge 65 commits into
base: main
Choose a base branch
from

Conversation

germa89
Copy link
Collaborator

@germa89 germa89 commented Nov 26, 2024

Description

As the title. Let's explore why the tests are taking so long since #3268

Issue linked

NA

Checklist

@germa89 germa89 requested a review from a team as a code owner November 26, 2024 10:57
@germa89 germa89 requested review from clatapie and pyansys-ci-bot and removed request for a team November 26, 2024 10:57
@germa89 germa89 self-assigned this Nov 26, 2024
@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

@github-actions github-actions bot added CI/CD Related with CICD, Github Actions, etc dependencies maintenance General maintenance of the repo (libraries, cicd, etc) labels Nov 26, 2024
Copy link

codecov bot commented Nov 26, 2024

Codecov Report

Attention: Patch coverage is 68.96552% with 9 lines in your changes missing coverage. Please review.

Project coverage is 87.11%. Comparing base (06c93c0) to head (afc448e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3577      +/-   ##
==========================================
- Coverage   87.13%   87.11%   -0.02%     
==========================================
  Files         187      187              
  Lines       14660    14677      +17     
==========================================
+ Hits        12774    12786      +12     
- Misses       1886     1891       +5     

@github-actions github-actions bot added the documentation Documentation related (improving, adding, etc) label Nov 27, 2024
tests/conftest.py Outdated Show resolved Hide resolved
tests/test_database.py Outdated Show resolved Hide resolved
tests/test_database.py Outdated Show resolved Hide resolved
tests/test_grpc.py Outdated Show resolved Hide resolved
tests/test_grpc.py Outdated Show resolved Hide resolved
tests/test_grpc.py Outdated Show resolved Hide resolved
@germa89 germa89 mentioned this pull request Dec 13, 2024
10 tasks
@germa89
Copy link
Collaborator Author

germa89 commented Dec 13, 2024

I'm renaming this PR.

The original issue #3591 was fixed in #3608

I am extracting the timeout option to #3621 so we separate concerns in different PRs.

However, there is a benefitial refactoring in the PR so I'm renaming it.

@germa89 germa89 changed the title ci: adding timeout to each test refactor: small improvements to test settings Dec 13, 2024
@github-actions github-actions bot added the enhancement Improve any current implemented feature label Dec 13, 2024
@germa89
Copy link
Collaborator Author

germa89 commented Dec 13, 2024

@germa89
Copy link
Collaborator Author

germa89 commented Dec 13, 2024

@pyansys-ci-bot LGTM.

Copy link
Contributor

@pyansys-ci-bot pyansys-ci-bot left a comment

Choose a reason for hiding this comment

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

✅ Approving this PR because germa89 said so in here 😬

LGTM

@germa89 germa89 marked this pull request as ready for review December 16, 2024 09:18
@germa89 germa89 enabled auto-merge (squash) December 19, 2024 17:35
# If there is no result file, this fails.
try:
nsets = int(self.nsets)
except MapdlRuntimeError:
Copy link
Contributor

Choose a reason for hiding this comment

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

@germa89 Don't you want to write something down in the log about it?

Comment on lines +150 to +152
info += f"\tNumber of result sets: {nsets}\n"
info += f"\tCurrent load step: {self.load_step}\n"
info += f"\tCurrent sub step: {self.sub_step}\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

sets, step, sub steps...
Remind me of the good old days with Mechanical. I am becoming rusty with it 😭 .


clear(mapdl)

# setup the full file
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# setup the full file
# set up the full file

mapdl.vmesh("all")

# Define a material (nominal steel in SI)
mapdl.mp("EX", 1, 210e9) # Elastic moduli in Pa (kg/(m*s**2))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
mapdl.mp("EX", 1, 210e9) # Elastic moduli in Pa (kg/(m*s**2))
mapdl.mp("EX", 1, 210e9) # Elastic modulus in Pa (kg/(m*s**2))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD Related with CICD, Github Actions, etc enhancement Improve any current implemented feature maintenance General maintenance of the repo (libraries, cicd, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants