+
Skip to content

Conversation

sylvestre
Copy link
Contributor

on the destination

Should fix tests/misc/xattr.sh

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/misc/xattr is no longer failing!

Copy link
Contributor

Choose a reason for hiding this comment

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

The following error message is shown when I run the tests: setfacl: a: No such file or directory. And all tests pass, even though I didn't apply the changes to cp.rs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed, thanks :)
Test improved!

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, the test still passes without the changes applied to cp.rs. Is that the expected behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, it failed on my system

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, works on my system too
let me look at 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.

the set command wasn't executed, it should be good now :)

---- test_cp::test_cp_preserve_xattr_readonly_source stdout ----
touch: /tmp/.tmpm2KYKU/a
run: /home/sylvestre/dev/debian/coreutils/target/debug/coreutils cp --preserve=xattr /tmp/.tmpm2KYKU/a /tmp/.tmpm2KYKU/e
thread 'test_cp::test_cp_preserve_xattr_readonly_source' panicked at tests/by-util/test_cp.rs:5988:10:
Command was expected to succeed. code: 1
stdout = 
 stderr = cp: Permission denied (os error 13)

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/misc/xattr is no longer failing!

Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/misc/xattr is no longer failing!
Congrats! The gnu test tests/seq/seq is no longer failing!

@sylvestre sylvestre requested a review from cakebaker December 31, 2024 14:25
Comment on lines 5941 to 5942
let xattr_key = "user.test";
let xattr_value = "value";
Copy link
Contributor

@cakebaker cakebaker Jan 3, 2025

Choose a reason for hiding this comment

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

As the test is relatively long it might make sense to get rid of these variables to improve the readability and use the values directly on lines 5946 and 5948.

Update: I think it only makes sense to get rid of xattr_value. See my other comment about the use of xattr_key.

Comment on lines 5978 to 5979
stdout.contains("user.test"),
"Expected 'user.test' not found in getfattr output:\n{}",
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you can use the xattr_key var you defined above.

Suggested change
stdout.contains("user.test"),
"Expected 'user.test' not found in getfattr output:\n{}",
stdout.contains(xattr_key),
"Expected '{xattr_key}' not found in getfattr output:\n{}",

Copy link
Contributor

@cakebaker cakebaker left a comment

Choose a reason for hiding this comment

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

Overall it looks good :)

Copy link

github-actions bot commented Jan 4, 2025

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/misc/xattr is no longer failing!

@cakebaker cakebaker merged commit 4ff48c3 into uutils:main Jan 5, 2025
65 checks passed
@cakebaker
Copy link
Contributor

Congrats! The gnu test tests/misc/xattr is no longer failing!

Cool :)

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

点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载