-
Notifications
You must be signed in to change notification settings - Fork 123
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
stress-ng: fix teardown #2910
stress-ng: fix teardown #2910
Conversation
generic/stress-ng.py
Outdated
sudo=True) | ||
if hasattr(self, 'loop_dev') and os.path.exists(self.loop_dev): | ||
process.run("umount %s" % self.loop_dev, ignore_status=True, sudo=True) | ||
process.run("losetup -d %s" % self.loop_dev, ignore_status=True, sudo=True) |
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.
@disgoel please take care of pep8 errors
# pycodestyle test.py
test.py:80:80: E501 line too long (82 > 79 characters)
test.py:117:80: E501 line too long (81 > 79 characters)
test.py:124:80: E501 line too long (87 > 79 characters)
test.py:173:80: E501 line too long (86 > 79 characters)
test.py:184:80: E501 line too long (86 > 79 characters)
test.py:202:80: E501 line too long (83 > 79 characters)
test.py:203:80: E501 line too long (87 > 79 characters)
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.
fixed pep8 errors
Fix the teardown function to prevent AttributeError by adding checks to see if the attribute and path exists before trying to remove it. Before fix: ERROR: 'Stressng' object has no attribute 'loop_dev' ERROR: 'Stressng' object has no attribute 'stressmnt' After fix: CANCEL: Build Failed, Please check the build logs for details !! Signed-off-by: Disha Goel <[email protected]>
a459773
to
8b8766c
Compare
process.run("rm -rf /tmp/blockfile", ignore_status=True, sudo=True) | ||
if (os.path.exists(self.stressmnt)): | ||
process.run(f"rm -rf {self.stressmnt}") | ||
if hasattr(self, 'stressmnt') and os.path.exists(self.stressmnt): |
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.
@disgoel is stressmnt
and /tmp/blockfile
are the same?
if so why are we using variable at one place and hardcoded value at another place?
prefer not to hard code as it may fail if variable name differs
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.
both are different
self.stressmnt is a directory and /tmp/blockfile is a block device
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.
@disgoel Thanks for PR.
LGTM
Fix the teardown function to prevent AttributeError by adding checks to see if the attribute and path exists before trying to remove it.
Before fix:
ERROR: 'Stressng' object has no attribute 'loop_dev'
ERROR: 'Stressng' object has no attribute 'stressmnt'
After fix:
CANCEL: Build Failed, Please check the build logs for details !!