-
Notifications
You must be signed in to change notification settings - Fork 0
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
Flexible working directory #31
base: develop
Are you sure you want to change the base?
Changes from 71 commits
2cfdbca
c47eca9
1b19f89
ab43881
1c68726
3fe5757
4476be7
1f898ee
fdc0805
392e875
c715679
30905af
31bf31c
d44100d
b61746f
bdd6f6a
c3545ed
fc94687
3d96768
823bcde
5c9568c
702c623
321e939
0c4bc3c
74bcf94
cc7e1b4
600cdf3
84fb6bd
0b139bb
0726e4f
79df117
95ffdff
ca55f31
1dc278e
60aa878
6d2dec0
7ed2796
1ac2802
273bb0f
7cd8981
e8b44fb
04f86d4
cb8d898
bc8dc9b
e2248e9
519925a
67f0d7b
29c5bc5
58c647d
79f1d7e
af7a39b
32ec918
47a91ae
4bfbe9b
6746efb
fa4c415
77008ef
1d31cfe
82f0e12
e7d832e
ec858b1
c9b1450
aa7bf8e
843d7d7
4b4911d
6434fc9
dbee11f
c6e7f93
d7ae45f
b1ccf7c
8348e5b
6e0caa7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,8 @@ | |
""" | ||
|
||
import glob | ||
import sys, logging, json, os, datetime | ||
import sys, logging, json, os, datetime, platform | ||
from pathlib import Path | ||
from typing import Union | ||
|
||
sys.path.append("./constrain") | ||
|
@@ -37,7 +38,6 @@ def __init__(self, workflow, run_workflow_now=False): | |
self.states = {} | ||
self.workflow_dict = {} | ||
self.running_sequence = [] | ||
|
||
# checking workflow validity is handed over to the API method. | ||
if isinstance(workflow, str): | ||
self.load_workflow_json(workflow) | ||
|
@@ -46,10 +46,68 @@ def __init__(self, workflow, run_workflow_now=False): | |
self.workflow_dict = workflow | ||
|
||
self.load_states() | ||
# change working dir | ||
self.change_work_dir() | ||
|
||
# run workflow now | ||
if run_workflow_now: | ||
self.run_workflow() | ||
|
||
def change_work_dir(self) -> None: | ||
# First, check if the workflow_dict has "working_dir" | ||
if "working_dir" not in self.workflow_dict: | ||
logging.info("No working_dir is specified") | ||
else: | ||
# Change the working path to the working_dir value in workflow_dict. | ||
if not isinstance(self.workflow_dict["working_dir"], str): | ||
# First, detect if the working dir is a valid string | ||
logging.error("working directory specified is not a valid string.") | ||
else: | ||
# Then detect if the working dir provided is in Linux format or Windows Format. | ||
if ("/" in self.workflow_dict["working_dir"]) and ( | ||
"\\" not in self.workflow_dict["working_dir"] | ||
): | ||
# in Linux Format | ||
if ( | ||
platform.system() == "Windows" | ||
): # convert it to the working platform if it is windows | ||
self.workflow_dict["working_dir"] = self.workflow_dict[ | ||
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. what is the reason of replacing 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. This part is supposed to detect if the working dir value specified is in Linux format(
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. I feel like that we don't need to manually translate the path format from Linux to Win or vise versa. Python interpreter can do this automatically. This means, either But I found. The correct Windows format path is ".\\path", while ".\path" will causes error because it will intepreted as escape sequences. . If you agree with this, I can move forward with removing this section of replacing |
||
"working_dir" | ||
].replace("/", "\\") | ||
logging.info("the working dir provided is in Linux format.") | ||
elif ("/" not in self.workflow_dict["working_dir"]) and ( | ||
"\\" in self.workflow_dict["working_dir"] | ||
): | ||
# in windows format | ||
if ( | ||
platform.system() == "Linux" | ||
): # convert it to the working platform if it is Linux | ||
self.workflow_dict["working_dir"] = self.workflow_dict[ | ||
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. similar to above, what is the reason for the replacement |
||
"working_dir" | ||
].replace("\\", "/") | ||
logging.info("the working dir provided is in Win format.") | ||
|
||
# change the working directory if it exists | ||
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. there are other scenarios for the working dir string. such as: 1) no 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. Yes. There are some scenarios with some characters(e.g. "", space) that might canse issues, and I am working to add tests for these to catch these potential issues. |
||
if os.path.exists(self.workflow_dict["working_dir"]): | ||
os.chdir(self.workflow_dict["working_dir"]) | ||
logging.info("Change current working path to the specified path.") | ||
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. I have added this part to create a new directory if it doesn't exist |
||
else: | ||
try: | ||
Path(self.workflow_dict["working_dir"]).mkdir( | ||
parents=True, exist_ok=True | ||
) | ||
logging.info( | ||
"working directory specified does not exist and create a new director." | ||
) | ||
os.chdir(self.workflow_dict["working_dir"]) | ||
except Exception as e: | ||
# If an invalid escapse sequence string is specified. E.g. ".\tests\api\test" | ||
if e.winerror == 123: | ||
# The error is : OSError: [WinError 123] The filename, directory name, or volume label syntax is incorrect. | ||
logging.error( | ||
"The working directory specified is an invalid escape sequence string." | ||
) | ||
|
||
def validate(self, verbose: bool = False) -> bool: | ||
"""function to be implemented to check for high level validity of the workflow definition""" | ||
workflow_schema = { | ||
|
@@ -301,6 +359,7 @@ def create_workflow_engine( | |
Returns: | ||
Union[None, WorkflowEngine]: Instantiated WorkflowEngine object if provided workflow is valid; None otherwise. | ||
""" | ||
|
||
if (isinstance(workflow, str) and os.path.isfile(workflow)) or isinstance( | ||
workflow, dict | ||
): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{"workflow_name": "A minimal case", "meta": {"author": "ConStrain Team", "date": "03/23/2024", "version": "1.0", "description": "A minimal case to test if flexible call feature works"}, "imports": [], "states": {}} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,294 @@ | ||
import unittest, sys, logging, json, os | ||
import platform | ||
from unittest.mock import patch | ||
|
||
import constrain | ||
from constrain.api import Workflow | ||
|
||
sys.path.append("./constrain") | ||
|
||
|
||
class TestFlexibleCalling(unittest.TestCase): | ||
def test_no_dir_provided(self): | ||
"""This test checks when no working directory is provided, | ||
if the program will behave correctly""" | ||
with self.assertLogs() as logobs: | ||
json_case_path = "./tests/api/data/flexible_calling_unit_test/verification_case_unit_test_Path.json" | ||
|
||
# Delete working_dir value in the json file | ||
with open(json_case_path, "r") as f: | ||
workflow_dict = json.load(f) | ||
workflow_dict.pop("working_dir", None) | ||
|
||
with open(json_case_path, "w") as f: | ||
json.dump(workflow_dict, f) | ||
|
||
workflow = Workflow(workflow=json_case_path) | ||
self.assertEqual( | ||
logobs.output[0], | ||
"INFO:root:No working_dir is specified", | ||
) | ||
|
||
def test_invalid_str(self): | ||
"""This test checks when working directory is not a valid string, | ||
if the program will behave correctly""" | ||
with self.assertLogs() as logobs: | ||
json_case_path = "./tests/api/data/flexible_calling_unit_test/verification_case_unit_test_Path.json" | ||
|
||
# Change working_dir value in the json file to a invalid string | ||
with open(json_case_path, "r") as f: | ||
workflow_dict = json.load(f) | ||
workflow_dict["working_dir"] = [] | ||
|
||
with open(json_case_path, "w") as f: | ||
json.dump(workflow_dict, f) | ||
|
||
workflow = Workflow(workflow=json_case_path) | ||
self.assertEqual( | ||
logobs.output[0], | ||
"ERROR:root:working directory specified is not a valid string.", | ||
) | ||
|
||
def test_Linux_path(self): | ||
"""This test check if the program can detect the working path provided is in Linux format.""" | ||
with self.assertLogs() as logobs: | ||
json_case_path = "./tests/api/data/flexible_calling_unit_test/verification_case_unit_test_Path.json" | ||
|
||
# Change working_dir value in the json file to a valid path in Linux format | ||
with open(json_case_path, "r") as f: | ||
workflow_dict = json.load(f) | ||
workflow_dict["working_dir"] = "./tests/api/result" | ||
|
||
with open(json_case_path, "w") as f: | ||
json.dump(workflow_dict, f) | ||
|
||
workflow = Workflow(workflow=json_case_path) | ||
# change current working directory back | ||
os.chdir("../../..") | ||
|
||
self.assertEqual( | ||
logobs.output[0], | ||
"INFO:root:the working dir provided is in Linux format.", | ||
) | ||
|
||
self.assertEqual( | ||
logobs.output[1], | ||
"INFO:root:Change current working path to the specified path.", | ||
) | ||
|
||
def test_Win_path(self): | ||
"""This test check if the program can detect the working path provided is in WIN format.""" | ||
with self.assertLogs() as logobs: | ||
json_case_path = "./tests/api/data/flexible_calling_unit_test/verification_case_unit_test_Path.json" | ||
|
||
# Change working_dir value in the json file to a valid path in Win format | ||
with open(json_case_path, "r") as f: | ||
workflow_dict = json.load(f) | ||
workflow_dict["working_dir"] = ".\\tests\\api\\result" | ||
|
||
with open(json_case_path, "w") as f: | ||
json.dump(workflow_dict, f) | ||
|
||
workflow = Workflow(workflow=json_case_path) | ||
# change current working directory back | ||
os.chdir("../../..") | ||
|
||
self.assertEqual( | ||
logobs.output[0], | ||
"INFO:root:the working dir provided is in Win format.", | ||
) | ||
self.assertEqual( | ||
logobs.output[1], | ||
"INFO:root:Change current working path to the specified path.", | ||
) | ||
|
||
def test_valid_dir(self): | ||
"""This test checks when a working directory is provided and it also points to the correct path, | ||
if the program will behave correctly""" | ||
with self.assertLogs() as logobs: | ||
json_case_path = "./tests/api/data/flexible_calling_unit_test/verification_case_unit_test_Path.json" | ||
|
||
# Change working_dir value in the json file to a valid path | ||
with open(json_case_path, "r") as f: | ||
workflow_dict = json.load(f) | ||
workflow_dict["working_dir"] = "./tests/api/result" | ||
|
||
with open(json_case_path, "w") as f: | ||
json.dump(workflow_dict, f) | ||
|
||
workflow = Workflow(workflow=json_case_path) | ||
|
||
# change current working directory back | ||
os.chdir("../../..") | ||
|
||
self.assertEqual( | ||
logobs.output[1], | ||
"INFO:root:Change current working path to the specified path.", | ||
) | ||
|
||
with self.assertLogs() as logobs: | ||
# Change working_dir value in the json file to a valid path in Win format | ||
with open(json_case_path, "r") as f: | ||
workflow_dict = json.load(f) | ||
workflow_dict["working_dir"] = ".\\tests\\api\\result" | ||
|
||
with open(json_case_path, "w") as f: | ||
json.dump(workflow_dict, f) | ||
|
||
workflow = Workflow(workflow=json_case_path) | ||
|
||
# Change current working directory back | ||
os.chdir("../../..") | ||
|
||
self.assertEqual( | ||
logobs.output[1], | ||
"INFO:root:Change current working path to the specified path.", | ||
) | ||
|
||
def test_dir_with_space(self): | ||
"""This test checks when a working directory with space is provided and it also points to the correct path, | ||
if the program will behave correctly""" | ||
with self.assertLogs() as logobs: | ||
json_case_path = "./tests/api/data/flexible_calling_unit_test/verification_case_unit_test_Path.json" | ||
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. seems like all tests are about relative path. Please also make sure it works with absolute path |
||
|
||
# Change working_dir value in the json file to a valid path | ||
with open(json_case_path, "r") as f: | ||
workflow_dict = json.load(f) | ||
workflow_dict["working_dir"] = "./tests/api/result/dir space" | ||
|
||
with open(json_case_path, "w") as f: | ||
json.dump(workflow_dict, f) | ||
|
||
workflow = Workflow(workflow=json_case_path) | ||
|
||
# change current working directory back | ||
os.chdir("../../../../") | ||
|
||
self.assertEqual( | ||
logobs.output[1], | ||
"INFO:root:Change current working path to the specified path.", | ||
) | ||
|
||
with self.assertLogs() as logobs: | ||
# Change working_dir value in the json file to a valid path in Win format | ||
with open(json_case_path, "r") as f: | ||
workflow_dict = json.load(f) | ||
workflow_dict["working_dir"] = ".\\tests\\api\\result\\dir space" | ||
|
||
with open(json_case_path, "w") as f: | ||
json.dump(workflow_dict, f) | ||
|
||
workflow = Workflow(workflow=json_case_path) | ||
|
||
# Change current working directory back | ||
os.chdir("../../../../") | ||
|
||
self.assertEqual( | ||
logobs.output[1], | ||
"INFO:root:Change current working path to the specified path.", | ||
) | ||
|
||
def test_dir_simple(self): | ||
"""This test checks when a simple working directory without any "\\" or "/" is provided and it also points to the correct path, | ||
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. Added this unittest for paths without |
||
if the program will behave correctly""" | ||
with self.assertLogs() as logobs: | ||
json_case_path = "./tests/api/data/flexible_calling_unit_test/verification_case_unit_test_Path.json" | ||
|
||
# Change working_dir value in the json file to a valid path | ||
with open(json_case_path, "r") as f: | ||
workflow_dict = json.load(f) | ||
workflow_dict["working_dir"] = "./tests" | ||
|
||
with open(json_case_path, "w") as f: | ||
json.dump(workflow_dict, f) | ||
|
||
workflow = Workflow(workflow=json_case_path) | ||
|
||
# change current working directory back | ||
os.chdir("../") | ||
|
||
self.assertEqual( | ||
logobs.output[1], | ||
"INFO:root:Change current working path to the specified path.", | ||
) | ||
|
||
with self.assertLogs() as logobs: | ||
# Change working_dir value in the json file to a valid path in Win format | ||
with open(json_case_path, "r") as f: | ||
workflow_dict = json.load(f) | ||
workflow_dict["working_dir"] = ".\\tests" | ||
|
||
with open(json_case_path, "w") as f: | ||
json.dump(workflow_dict, f) | ||
|
||
workflow = Workflow(workflow=json_case_path) | ||
|
||
# Change current working directory back | ||
os.chdir("../") | ||
|
||
self.assertEqual( | ||
logobs.output[1], | ||
"INFO:root:Change current working path to the specified path.", | ||
) | ||
|
||
def test_valid_absolute_dir(self): | ||
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. Add this test for absolute path |
||
"""This test checks when a absolute working directory is provided and it also points to the correct path, | ||
if the program will behave correctly""" | ||
with self.assertLogs() as logobs: | ||
json_case_path = "./tests/api/data/flexible_calling_unit_test/verification_case_unit_test_Path.json" | ||
|
||
# Change working_dir value in the json file to a valid path | ||
print(os.getcwd()) | ||
|
||
with open(json_case_path, "r") as f: | ||
workflow_dict = json.load(f) | ||
if platform.system() == "Windows": | ||
workflow_dict["working_dir"] = os.getcwd() + "\\tests\\api\\result" | ||
else: | ||
workflow_dict["working_dir"] = os.getcwd() + "/tests/api/result" | ||
|
||
with open(json_case_path, "w") as f: | ||
json.dump(workflow_dict, f) | ||
|
||
workflow = Workflow(workflow=json_case_path) | ||
|
||
# change current working directory back | ||
os.chdir("../../..") | ||
|
||
self.assertEqual( | ||
logobs.output[1], | ||
"INFO:root:Change current working path to the specified path.", | ||
) | ||
|
||
def test_dir_not_exist(self): | ||
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. Add this test for path that doesn't exist |
||
"""This test checks when a valid wd is provided but it doesn't exist, | ||
if the program will behave correctly""" | ||
with self.assertLogs() as logobs: | ||
json_case_path = "./tests/api/data/flexible_calling_unit_test/verification_case_unit_test_Path.json" | ||
|
||
# Change working_dir value in the json file to a path that does not exist | ||
with open(json_case_path, "r") as f: | ||
workflow_dict = json.load(f) | ||
workflow_dict["working_dir"] = "./tests/api/result/not_existing_path" | ||
|
||
with open(json_case_path, "w") as f: | ||
json.dump(workflow_dict, f) | ||
|
||
workflow = Workflow(workflow=json_case_path) | ||
# change current working directory back | ||
os.chdir("../../../../") | ||
|
||
# then delete this path. | ||
# print(os.getcwd()) | ||
|
||
# But I always get error deleting this directory: | ||
# os.remove("./tests/api/result/not_existing_path/test") | ||
|
||
self.assertEqual( | ||
logobs.output[1], | ||
"INFO:root:working directory specified does not exist and create a new director.", | ||
) | ||
|
||
|
||
if __name__ == "__main__": | ||
unittest.main() |
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.
Please put the whole working dir logic into a dedicated method
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.
I have moved this whole working dir logic to a dedicated method.