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

test: Improving conftest.py so that local configuration is handled #187

Merged
merged 17 commits into from
Dec 16, 2024

Conversation

a-bouth
Copy link
Collaborator

@a-bouth a-bouth commented Dec 6, 2024

Adapted conftest.py in such way that it is transparent for the user to use whether he's in local or "docker" configuration with the DPF Server.

Main Changes

  • We use a gRPC configuration for the local server and make it global, in connect_to_or_start_server
  • All tests files are now uploaded on the server side and the paths are uploaded accordingly, the path will always be the correct one either in local or docker configuration
  • Exception made for flute_psd.txt this file is needed only locally as we're using native python functions to read it and not DPF Operators
  • The fixture dpf_sound_test_server is not needed anymore, everything is done in pytest_configure
  • The functions get_absolute_path_for* are not relevant anymore and caused maintainability problems so I removed them (and thus removed the file _get_example_file.py, don't panick, there's still download.py that collect files for our examples and is still very relevant)

Side Changes

  • In source_control_spectrum I had a text formatting error locally, I changed the syntax but the result is the same
  • In tests for prominence_ratio and tone_to_noise_ratio there was a missing fixture show_mockup for plot tests that caused a bug, I added them
  • In tests for prominence_ratio and tone_to_noise_ratio I changed the way we obtained `flute_psd.txt

Doc

https://dpf.docs.pyansys.com/version/stable/getting_started/dpf_server.html

image

@a-bouth a-bouth linked an issue Dec 6, 2024 that may be closed by this pull request
@github-actions github-actions bot added testing enhancement New features or code improvements labels Dec 6, 2024
@github-actions github-actions bot added testing and removed testing labels Dec 6, 2024
@github-actions github-actions bot added testing and removed testing labels Dec 6, 2024
@github-actions github-actions bot added testing and removed testing labels Dec 6, 2024
@a-bouth a-bouth marked this pull request as ready for review December 6, 2024 14:26
@github-actions github-actions bot added testing and removed testing labels Dec 6, 2024
Copy link
Collaborator

@ansaminard ansaminard left a comment

Choose a reason for hiding this comment

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

Just a minor suggestion with no functional impact.
Also a question, will that allow us to add tests for existence of input files in feature classes? ( I remember we ran into an issue before when doing such tests because of the ambiguity of the file location between the host machine and the server)

tests/conftest.py Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the testing label Dec 9, 2024
@a-bouth
Copy link
Collaborator Author

a-bouth commented Dec 9, 2024

Just a minor suggestion with no functional impact. Also a question, will that allow us to add tests for existence of input files in feature classes? ( I remember we ran into an issue before when doing such tests because of the ambiguity of the file location between the host machine and the server)

@ansaminard probably no, because the feature classes should not be aware of whether they're in "Docker" or "local" configuration

With the changes I've done in my PR, we know where the files are as long as we have access to the server variable

I'm trying to think of a way of doing that while writing those lines but all solutions that come to mind seems way too complex

For the tests and the documentation, we're in a controlled environment, with as many input file paths as we have users, this will probably be too complex

@a-bouth a-bouth enabled auto-merge (squash) December 9, 2024 16:26
@a-bouth a-bouth disabled auto-merge December 13, 2024 10:52
@github-actions github-actions bot added testing and removed testing labels Dec 13, 2024
anshlachamb
anshlachamb previously approved these changes Dec 13, 2024
@github-actions github-actions bot removed the testing label Dec 13, 2024
@github-actions github-actions bot added testing and removed testing labels Dec 13, 2024
@a-bouth a-bouth merged commit cb4d73d into main Dec 16, 2024
31 checks passed
@a-bouth a-bouth deleted the feat/181-add-local-server-configuration-in-conftestpy branch December 16, 2024 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features or code improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add local server configuration in conftest.py
4 participants