Fix Unpacking with merge flag failed without --dir flag

[tommady]

close #825

[marcospb19]

Hey, apologies for the huge delay, I'm back from a break. This is marked as a draft, is it ready for review now? :)

[tommady]

Hey, apologies for the huge delay, I'm back from a break. This is marked as a draft, is it ready for review now? :)

Hey, I am sorry, I forgot this draft PR...
After reading my last work, I recall the problem I was trying to solve.

As you can see, the integration test changes,
I followed the issue, but the integration test cannot be reproduced when you undo the other changes I made.

I'll commit to identifying the problem if the words cannot be clearly understood.

[tommady]

ok, right now ignore the Clippy error, the updated test case passed...

test unpack_multiple_sources_into_the_same_destination_with_merge ... ok

And I am confused.

[marcospb19]

Hah, did you expect it to fail but it's passing?

I triggered a re-run just in case 👀

[marcospb19]

Do you get the same error when running locally? #798 (comment)

[tommady]

Do you get the same error when running locally? #798 (comment)

Yes, I can reproduce it locally, even:

1. sudo
2. under tmp folder

But the integration test cannot simulate this issue

[marcospb19]

But the integration test cannot simulate this issue

@tommady I figured the tests don't cover this bug because the bug itself only happens when the --dir or -d flag is not present, and all the tests rely on this -d flag so that it doesn't pollute the current dir with test artifacts, but instead, write them to /tmp.

Unfortunately, due to the way cargo test runner works, we also can't use std::env::set_current_dir to avoid this.

So there are 2 ways forward I can think of:

1. Not having tests that cover this specific bug when -d is not present.
2. Accepting test artifacts to current folder.

I'm fine with 1 I guess, what do you think?

[tommady]

But the integration test cannot simulate this issue

@tommady I figured the tests don't cover this bug because the bug itself only happens when the --dir or -d flag is not present, and all the tests rely on this -d flag so that it doesn't pollute the current dir with test artifacts, but instead, write them to /tmp.

Unfortunately, due to the way cargo test runner works, we also can't use std::env::set_current_dir to avoid this.

So there are 2 ways forward I can think of:

1. Not having tests that cover this specific bug when -d is not present.
2. Accepting test artifacts to current folder.

I'm fine with 1 I guess, what do you think?

interesting.
anyway thanks for your explanation
now I see this issue more clearly, thanks!

I'll try to do the option 1
thanks.

[tommady]

hi @marcospb19
could you kindly do a review while you have time?
thanks.

[marcospb19]

Great! Thanks for sticking to this, really appreciate it.