Sort out skipped trashbin integration tests

[phil-davis]

Description

1. tag the existing skipped tests with issue-23151
2. add a new step definition that will wait and delete, to work around the timing issue
3. use that step definition in new scenarios that demonstrate the good working behaviour

Related Issue

#23151

Motivation and Context

I noticed 2 trashbin tests that were skipped, while refactoring integration tests.

I tried the tests, and they pass locally for me, they look decent and I tried combinations of what they do to make sure they would really fail if things are not right. But they fail when run in CI. It turns out they fail when run quickly - if multiple files with the same name are deleted in the same second then there is a naming conflict in the trashbin.

The automated tests can have 1 scenario that work around the timing bug/issue and tests/demonstrates the other correct behaviour of putting files with the same name from different folders into the trashbin. Then the existing scenario with the timing bug/issue can remain skipped, but also be tagged with the issue number so people can easily follow it up in future.

How Has This Been Tested?

Run the skipped tests locally - they pass
Unskip the tests to run them in CI - they fail there, so we know they are "good" example failing cases

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[phil-davis]

These were skipped in #28111
But (assuming CI passes here) they work now.

Edit: this comment is now out-of-date - see updated first post for full details of the proposed "solution".

[codecov]

Codecov Report

Merging #30389 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #30389   +/-   ##
=========================================
  Coverage     61.61%   61.61%           
  Complexity    18507    18507           
=========================================
  Files          1090     1090           
  Lines         61112    61112           
=========================================
  Hits          37653    37653           
  Misses        23459    23459

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1d0f2f...07e87e6. Read the comment docs.

[phil-davis]

Hmmm - those tests fail on Jenkins, but pass for me locally. So there is something about the Jenkins environment/timing and the details of this test that needs looking into.

[PVince81]

@phil-davis please note that deleted files get a file name like "test.txt.d1234567" where the number is a timestamp. Maybe if it deletes too fast the timestamp is exactly the same so we bump into this issue: #23151

[phil-davis]

Thanks for the hint - I will get back to this soon...

[phil-davis]

@SergioBertolinSG @individual-it @PVince81 I have updated the code and squashed.
This keeps the failing scenarios, better tagging them against the relevant issue, and provides new scenarios that at least test/demonstrate the good behaviour that is there.

Please review again.

code has changed significantly

[phil-davis]

Backport stable10 #23151

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.