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

Flexible working directory #31

Open
wants to merge 72 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
2cfdbca
Commit test for Issue 16
Fan-Feng Mar 1, 2024
c47eca9
update for Issue16
Fan-Feng Mar 5, 2024
1b19f89
Add ctest for Linux/Windows Working path
Fan-Feng Mar 12, 2024
ab43881
add check for working directory format
Fan-Feng Mar 13, 2024
1c68726
add check for Working path format
Fan-Feng Mar 18, 2024
3fe5757
fix formatting error
Fan-Feng Mar 20, 2024
4476be7
fix formatting error
Fan-Feng Mar 20, 2024
1f898ee
fix formatting error
Fan-Feng Mar 20, 2024
fdc0805
fix formatting error
Fan-Feng Mar 20, 2024
392e875
fix formatting error
Fan-Feng Mar 20, 2024
c715679
fix formatting error
Fan-Feng Mar 20, 2024
30905af
fix formatting error
Fan-Feng Mar 20, 2024
31bf31c
add unit test
Fan-Feng Mar 22, 2024
d44100d
add unittest if path does not exist
Fan-Feng Mar 22, 2024
b61746f
unittest for flexible calling
Fan-Feng Mar 22, 2024
bdd6f6a
fix formatting error
Fan-Feng Mar 22, 2024
c3545ed
Update unittest for flexible calling
Fan-Feng Mar 25, 2024
fc94687
add a minimal case for unit test
Fan-Feng Mar 25, 2024
3d96768
fix unit test
Fan-Feng Mar 25, 2024
823bcde
fix unit test
Fan-Feng Mar 25, 2024
5c9568c
Fix unitttest
Fan-Feng Mar 25, 2024
702c623
fix unittest
Fan-Feng Mar 25, 2024
321e939
fix unittest
Fan-Feng Mar 25, 2024
0c4bc3c
test unittest
Fan-Feng Mar 25, 2024
74bcf94
Test for unit test 1
Fan-Feng Mar 25, 2024
cc7e1b4
Fix unittest
Fan-Feng Mar 25, 2024
600cdf3
Fix unittest
Fan-Feng Mar 25, 2024
84fb6bd
Fix unit test
Fan-Feng Mar 25, 2024
0b139bb
Test1
Fan-Feng Mar 25, 2024
0726e4f
Test1
Fan-Feng Mar 25, 2024
79df117
Fix unittest
Fan-Feng Mar 25, 2024
95ffdff
Unit test 1
Fan-Feng Mar 25, 2024
ca55f31
Fix unittest
Fan-Feng Mar 25, 2024
1dc278e
Unit test 1
Fan-Feng Mar 25, 2024
60aa878
Move flexible calling to a new folder
Fan-Feng Mar 25, 2024
6d2dec0
Update unittest for flexible calling
Fan-Feng Mar 25, 2024
7ed2796
clean redundant comments
Fan-Feng Apr 16, 2024
1ac2802
clean redundant comments
Fan-Feng Apr 16, 2024
273bb0f
create json schemas for library and verification case
Fan-Feng May 8, 2024
7cd8981
Create a new directory if it doesn't exist
Fan-Feng May 15, 2024
e8b44fb
put the whole working dir logic into a dedicate
Fan-Feng May 15, 2024
04f86d4
fix unittest error
Fan-Feng May 15, 2024
cb8d898
Merge branch 'CODEVERIFY-16' of https://github.com/pnnl/ConStrain int…
Fan-Feng May 15, 2024
bc8dc9b
rm json schema
Fan-Feng May 15, 2024
e2248e9
Fix unittest error
Fan-Feng May 16, 2024
519925a
Black formatting
Fan-Feng May 16, 2024
67f0d7b
black formatting--weird. This file is already reformatted on my compu…
Fan-Feng May 16, 2024
29c5bc5
add unittest for absolute path
Fan-Feng May 17, 2024
58c647d
Add checks based on Jerry's comments
May 30, 2024
79f1d7e
Fix unittest error
Fan-Feng May 30, 2024
af7a39b
Black format
Fan-Feng May 30, 2024
32ec918
Fix unittest issue
Fan-Feng May 30, 2024
47a91ae
revise json file
Fan-Feng May 30, 2024
4bfbe9b
fix unittest
Fan-Feng May 30, 2024
6746efb
Add unittest for invalid working directories
Fan-Feng May 30, 2024
fa4c415
add chc for linux-format path
Fan-Feng May 30, 2024
77008ef
add check for Win-format path
Fan-Feng May 30, 2024
1d31cfe
fix windows_format path
Fan-Feng May 31, 2024
82f0e12
Fix windows format path
Fan-Feng May 31, 2024
e7d832e
Add checfr escape sequence path
Fan-Feng May 31, 2024
ec858b1
add checks for paths with space
Fan-Feng May 31, 2024
c9b1450
Add check for path with space
Fan-Feng May 31, 2024
aa7bf8e
add check for absolute path
Fan-Feng May 31, 2024
843d7d7
fix check for absolute path
Fan-Feng May 31, 2024
4b4911d
Fix uittest for Absolute path
Fan-Feng May 31, 2024
6434fc9
Black formatting
Fan-Feng May 31, 2024
dbee11f
Add unittest for Invalid string with escape sequence path
Fan-Feng May 31, 2024
c6e7f93
Fix unittest for invalid strings with escape sequences
Fan-Feng May 31, 2024
d7ae45f
Test escape sequence
Fan-Feng May 31, 2024
b1ccf7c
add test for path that does not exist
Fan-Feng May 31, 2024
8348e5b
Black formatting
Fan-Feng May 31, 2024
6e0caa7
add unit test for path without \ or /
Fan-Feng Sep 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 42 additions & 2 deletions constrain/api/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"""

import glob
import sys, logging, json, os, datetime
import sys, logging, json, os, datetime, platform
from typing import Union

sys.path.append("./constrain")
Expand Down Expand Up @@ -37,7 +37,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)
Expand All @@ -47,6 +46,46 @@ def __init__(self, workflow, run_workflow_now=False):

self.load_states()

# First, check if the workflow_dict has "working_dir"
Copy link
Collaborator

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

Copy link
Collaborator Author

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.

if "working_dir" not in self.workflow_dict:
logging.info("No working_dic 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[
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the reason of replacing \ with \\

Copy link
Collaborator Author

@Fan-Feng Fan-Feng Apr 17, 2024

Choose a reason for hiding this comment

The 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( / ) or Windows format (\\).

  • If the working dir specified is in Linux format (/), and the platform is Windows (\\), then replace / with \\.
  • If the working dir specified is in Win format (\\), and the platform is Linux(/), then replace \\ with /.

Copy link
Collaborator Author

@Fan-Feng Fan-Feng May 29, 2024

Choose a reason for hiding this comment

The 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 .\\path or "./path" is valid path for both Linux and Windows machine.

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 \\ and "/", and add a check for ".\path".

"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[
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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 \\ and no /, 2)spaces causing potential issues in mkdir (if we choose to do so); 3) having single \ in a Window format. etc

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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.")
Copy link
Collaborator Author

@Fan-Feng Fan-Feng May 31, 2024

Choose a reason for hiding this comment

The 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:
logging.error("working directory specified does not exist.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did we say if we want to create a new dir if it does not exist?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't remember you asking me to make a new dir if it does not exist. I can add this functionality if it is needed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added.


# run workflow now
if run_workflow_now:
self.run_workflow()

Expand Down Expand Up @@ -301,6 +340,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
):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"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": {}
}
160 changes: 160 additions & 0 deletions tests/api/test_flexible_calling.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
import unittest, sys, logging, json, os
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)
del 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],
"INFO:root:No working_dic 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.",
)

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.",
)

def test_dir_not_exist(self):
"""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"] = "./not_existing_path/test"

with open(json_case_path, "w") as f:
json.dump(workflow_dict, f)

workflow = Workflow(workflow=json_case_path)
self.assertEqual(
logobs.output[1],
"ERROR:root:working directory specified does not exist.",
)

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"
Copy link
Collaborator

Choose a reason for hiding this comment

The 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"

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.",
)


if __name__ == "__main__":
unittest.main()
Loading