Skip to content

ROX-31266: Implement tests with valid and invalid utf 8 strings#251

Open
JoukoVirtanen wants to merge 6 commits intomainfrom
jv-ROX-31266-implement-tests-with-valid-and-invalid-utf-8-strings
Open

ROX-31266: Implement tests with valid and invalid utf 8 strings#251
JoukoVirtanen wants to merge 6 commits intomainfrom
jv-ROX-31266-implement-tests-with-valid-and-invalid-utf-8-strings

Conversation

@JoukoVirtanen
Copy link
Contributor

Description

Parameterized the filename in the existing test_open and test_multiple tests. The input file names include regular ASCII, accents, Cyrillic, Chinese, and emoji.

Unit tests were also added for slice_to_string, sanitize_d_path, and creation of processes with valid ASCII, Cyrillic, Chinese, Japanese, Arabic, emoji, and invalid UTF-8.

Checklist

  • Investigated and inspected CI test results
  • Updated documentation accordingly

Automated testing

  • Added unit tests
  • Added integration tests
  • Added regression tests

If any of these don't apply, please comment below.

Testing Performed

CI is sufficient

@JoukoVirtanen JoukoVirtanen marked this pull request as ready for review February 6, 2026 06:32
@JoukoVirtanen JoukoVirtanen requested a review from a team as a code owner February 6, 2026 06:32
Copy link
Contributor

@Molter73 Molter73 left a comment

Choose a reason for hiding this comment

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

I still need to go through the rust unit tests, my first impression is that they are quite convoluted for testing some functions that are relatively simple and straightforward. The integration tests provide a lot more value because they are testing not just those functions but also the BPF programs and the rest of the userspace logic, I would focus on those.

['0.txt', '1.txt', '2.txt'],
['café.txt', 'файл.txt', '测试.txt'],
])
def test_multiple(fact, monitored_dir, server, filenames):
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if there is much value in this change. This test is checking that multiple events are captured, so the actual paths used are not that important. Maybe we want to rollback this part?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted

'测试.txt',
'🚀rocket.txt',
])
def test_open(fact, monitored_dir, server, filename):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we expand this to the tests on the other LSM hooks? Namely test_path_unlink.py, test_path_chmod.py and test_path_chown.py.

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.

2 participants