RemoveDir returns !HasLoggedErrors

[benvillalobos]

Context

RemoveDir doesn't follow the typical task convention that returns "I didn't log any errors." Instead, RemoveDir returned "I actually deleted all directories," which can be checked via the RemovedDirectories output item.

Changes Made

RemoveDir now returns !Log.HasLoggedErrors.

Testing

I noticed RemoveDir has a total of one test. Added another "basic" test while I'm here.

Notes

I removed the concept of an "overall success" because you can deduce it from the output item and the collection passed in. If we should keep it, I can add a boolean output parameter for "overallsuccess."

[ladipro]

Are there scenarios where the task will not return the same value with this change? It would seem that whenever it was logging an error it was also setting currentSuccess to false so the behavior should be equivalent. Thank you!

[benvillalobos]

Are there scenarios where the task will not return the same value with this change?

The case that this would "break" would be something like a build expecting to do something when removedir fails to delete a directory. The correct way to check that would be to check the number of directories passed in vs the number of directories deleted; RemovedDirectories

[ladipro]

Doesn't failing to delete a directory always log an error, though? Just curious, apologies for the stupid question.

[benvillalobos]

Doesn't failing to delete a directory always log an error, though?

I recall RemoveDir being surprisingly good about logging errors when a deletion failed and tried to return false whenever that happened. For a second I forgot what this PR was solving. Curse the question that forces me to remind myself of the full context here 😁

This is an tangential PR from #6912. 6912 solves the real problem that stemmed from #6275 (tasks & taskbuilders were out of sync in understanding that a task "officially" logged an error when tasks log Error to Warning conversions)

This PR just ensures we keep our own "best practices." So yeah, this is a preventative measure and there shouldn't (🤞) be a visible change.