Skip to content

Conversation

@ben-grande
Copy link
Contributor

@ben-grande ben-grande commented May 3, 2025

Changes

Add tests for safe terminal utilities.

Mandatory Checklist

  • Legal agreements accepted. By contributing to this organisation, you acknowledge you have read, understood, and agree to be bound by these these agreements:

Terms of Service, Privacy Policy, Cookie Policy, E-Sign Consent, DMCA, Imprint

Optional Checklist

The following items are optional but might be requested in certain cases.

  • I have tested it locally
  • I have reviewed and updated any documentation if relevant
  • I am providing new code and test(s) for it

Fixes:

@ben-grande ben-grande force-pushed the st-test branch 2 times, most recently from 77fae4e to 4318162 Compare May 5, 2025 13:52
- Check output when no argument is provided;
- Check if stdin is ignored by default;
- Check if stdin is ignored if file is provided;
- Check how many arguments a utility accepts;
- Check how word splitting works on multiple arguments;
- Check if newline is present; and
- How it treats the argument '-'.
@ben-grande ben-grande force-pushed the st-test branch 7 times, most recently from 0cc3cbe to 2e92192 Compare May 5, 2025 15:30
Copy link
Contributor

@ArrayBolt3 ArrayBolt3 left a comment

Choose a reason for hiding this comment

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

Tests pass both as-is and with the couple of functional changes suggested.

Comment on lines 34 to 51
i = 0
self.tmpdir = tempfile.mkdtemp()
while i < 6:
self.tmpfiles_list.append(os.path.join(self.tmpdir, str(i)))
with open(self.tmpfiles_list[i], "w", encoding="utf-8") as file:
if i == 0:
file.write("")
elif i == 1:
file.write("".join(contents))
elif i == 2:
file.write("".join(contents) + "\n")
elif i == 3:
file.write(self.text_dirty)
elif i in [4, 5]:
pass
file.flush()
file.close()
i += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
i = 0
self.tmpdir = tempfile.mkdtemp()
while i < 6:
self.tmpfiles_list.append(os.path.join(self.tmpdir, str(i)))
with open(self.tmpfiles_list[i], "w", encoding="utf-8") as file:
if i == 0:
file.write("")
elif i == 1:
file.write("".join(contents))
elif i == 2:
file.write("".join(contents) + "\n")
elif i == 3:
file.write(self.text_dirty)
elif i in [4, 5]:
pass
file.flush()
file.close()
i += 1
self.tmpdir = tempfile.mkdtemp()
for i in range(0, 6):
self.tmpfiles_list.append(os.path.join(self.tmpdir, str(i)))
with open(self.tmpfiles_list[i], "w", encoding="utf-8") as file:
if i == 0:
file.write("")
elif i == 1:
file.write("".join(contents))
elif i == 2:
file.write("".join(contents) + "\n")
elif i == 3:
file.write(self.text_dirty)
elif i in [4, 5]:
pass
file.flush()
file.close()

Copy link
Contributor Author

@ben-grande ben-grande May 11, 2025

Choose a reason for hiding this comment

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

Github browser diff is bad on this case. Is the change while to for and removal of the individual i var assignments?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the change while to for and removal of the individual i var assignments?

Yes.

Github browser diff is bad on this case.

Yeah... not sure why it didn't render this very well this time.

shutil.rmtree(self.tmpdir)

def _del_module(self) -> None:
for module in ["stdisplay." + self.module]: # type: ignore # pylint: disable=no-member
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be useful to document that self.module is going to be defined in the individual tests themselves.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 95 to 101
def _get_file(self, file: str) -> str:
"""
Helper function get contents of a file.
"""
with open(file, mode="r", encoding="utf-8") as fileobj:
text = fileobj.read()
return text
Copy link
Contributor

Choose a reason for hiding this comment

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

Could Path.read_text() be used instead of a custom helper function for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Done.

import stdisplay.tests


class TestSTCat(stdisplay.tests.TestSTBase):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
class TestSTCat(stdisplay.tests.TestSTBase):
class TestSTCatn(stdisplay.tests.TestSTBase):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

class TestSTPrint(stdisplay.tests.TestSTBase):
"""
Test stprint
Test stecho.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Test stecho.
Test stprint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

"",
self._get_file(file=self.tmpfiles["fill"]),
)
# Empty stdin when writing to file and file sanitization.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Empty stdin when writing to file and file sanitization.
# Empty stdout when writing to file and file sanitization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

self.text_dirty_sanitized,
self._get_file(file=self.tmpfiles["fill"]),
)
# Empty stdin when writing to multiple files and its sanitization.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Empty stdin when writing to multiple files and its sanitization.
# Empty stdout when writing to multiple files and its sanitization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@adrelanos adrelanos merged commit be407ed into Kicksecure:master May 13, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants