-
Notifications
You must be signed in to change notification settings - Fork 35
feat: added pytest fixture to create and use its own temporary directory structure #363
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
Draft
rohansen856
wants to merge
7
commits into
zowe:main
Choose a base branch
from
rohansen856:test-not-dependent-on-env
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
7a369d5
feat: added pytest fixture to create and use its own temporary direct…
rohansen856 699a81d
Merge branch 'main' into test-not-dependent-on-env
rohansen856 b342ee1
fix: moved conftest file and initialized isolation class
rohansen856 7757c2e
test: updated isolated env test
rohansen856 ae79b01
enhancement: created a central class for inheriting unittest Testcase…
rohansen856 e2ea9d9
Merge branch 'main' into test-not-dependent-on-env
rohansen856 fc9df2c
Merge branch 'main' into test-not-dependent-on-env
zFernand0 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| import pytest | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def isolated_test_env(tmp_path): | ||
| project_dir = tmp_path / "project" | ||
| global_dir = tmp_path / "global" | ||
| project_dir.mkdir() | ||
| global_dir.mkdir() | ||
|
|
||
| return { | ||
| "project_dir": project_dir, | ||
| "global_dir": global_dir | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| from pathlib import Path | ||
|
|
||
|
|
||
| def test_isolated_env_does_not_touch_real_home(isolated_test_env): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| project_dir = isolated_test_env["project_dir"] | ||
| global_dir = isolated_test_env["global_dir"] | ||
|
|
||
| # Copy contets of the actual zowe.config.json as it's necessary for the test | ||
| real_config_path = Path(__file__).resolve().parent.parent.parent / "zowe.config.json" | ||
|
|
||
| assert real_config_path.exists(), f"Missing config at {real_config_path}" | ||
|
|
||
| config_file = project_dir / "zowe.config.json" | ||
| config_file.write_text(real_config_path.read_text()) | ||
|
|
||
| assert config_file.exists() | ||
| assert "profiles" in config_file.read_text() | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for keeping the globa vs. project configs here...
I believe that for the purpose of integration testing we could simplify this to just use the
tmp_path / zowe.config.jsonUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zFernand0 @pem70 I think instead of
tmp_pathwe need to return theconfig_fileandconfig_pathvariables from the function. It would look something like this:And for the other file (
tests/integration/test_integration_env.py):And yes this simplifies the overall structure by isolating all the logic and assertions in different functions🫡. Please let me know if I am mistaken anywhere...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is alright then we can possibly move towards making the structure more modular by using it like this:
class TestIsolatedEnv(unittest.TestCase):as you suggested. A step by step approach would be better so I would like to first make a commit for modifications as mentioned above then move towards the modular integration...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zFernand0 is it ok to require
zowe.config.jsonto be at the root level of the project, or should we allow it to reside at any folder level?