Add test for remote worker path resolution#97
Conversation
d3dfa6b to
761121e
Compare
Szelethus
left a comment
There was a problem hiding this comment.
I'm fine with creating/removing the tmp directory with a class level setup/teardown, but by having a global PATH_RESOLUTION, you practically do 90% of the test in those methods. In fact, I'm not even sure the way you are testing there is a need for creating any files. I recommend that you simply test the string replace, which is all that really matters here.
src/codechecker_script.py
Outdated
There was a problem hiding this comment.
This is a very intrusive test. You are not testing the entirety of this function, only this part:
data = data_file.read()
for pattern, replace in BAZEL_PATHS.items():
data = re.sub(pattern, replace, data)
Lets just create a separate function that takes some string and returns with a substituted one. One something like that. That way you won't need to add an intrusive parameter just for testing.
There was a problem hiding this comment.
We could just import the list of regexes from here, to not be so intrusive.
bea98fe to
7d4745a
Compare
Szelethus
left a comment
There was a problem hiding this comment.
Awesome, this is what I wanted to see! I have some nits, but otherwise this is a great PR.
There was a problem hiding this comment.
Why do we need this global variable? We could just pass each of these as an argument.
There was a problem hiding this comment.
I thought it would be easier to test other methods we might add in the future, this way.
1a9a825 to
5d312a2
Compare
Why:
We want to test our solutions for making remote workers' absolute paths into locally usable relative paths. We should add a test for it.
What:
Adds a new test to run on pre-created remote worker paths.
Addresses:
#65