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 wildcards #98

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

forsyth2
Copy link
Collaborator

@forsyth2 forsyth2 commented Oct 1, 2020

Add test for wildcards, specifically the scenario presented in #96.

@forsyth2 forsyth2 added the Testing Files in `tests` modified label Oct 1, 2020
@forsyth2 forsyth2 requested review from golaz and TonyB9000 October 1, 2020 22:31
@forsyth2 forsyth2 self-assigned this Oct 1, 2020
@@ -148,6 +187,9 @@ def testExtractKeepHPSS(self):
def testExtractCache(self):
self.helperExtractCache('testExtractCache', 'none')

def testExtractWildcard(self):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we need a test with HPSS too?

print(output + err)
expected_present = ['Extracting ab2.txt', 'Extracting ab3.txt']
self.check_strings(cmd, output + err, expected_present, [])
cmd = '{}zstash extract --hpss={} a*'.format(zstash_path, self.hpss_path)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interestingly, adding double quotes seems to cause a failure and not adding them allows the tests to pass -- the reverse of our experience as users. I believe this is because subprocess.Popen requires cmd to be converted to a list and I imagine the conversion encapsulates a* in quotes. Not sure if there's a way to automate a test in such a way as to use the quotes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Try using shell=True? If that doesn't work, perhaps we should just close #96 without adding automated testing.

@forsyth2 forsyth2 added the priority: low Low priority task label Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low Low priority task Testing Files in `tests` modified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant