Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up test intermediates when clean test rebuild is requested #62001

Merged
merged 2 commits into from
Nov 24, 2021

Conversation

trylek
Copy link
Member

@trylek trylek commented Nov 24, 2021

I hit this problem locally when working on the test merging -
I made a change in Jeremy's Roslyn generator logic adding a hard
FailFast and the tests continued building normally even when
specifying the rebuild option because the XUnitWrapperGenerator
project remained in the intermediate test folder and didn't get
actually rebuilt. As an additional tiny cleanup I have also replaced
__TestWorkingDir with __TestBinDir in the Unix version of the script
as they are always identical.

Thanks

Tomas

/cc @dotnet/runtime-infrastructure

I hit this problem locally when working on the test merging
- I made a change in Jeremy's Roslyn generator logic adding a hard
FailFast and the tests continued building normally even when
specifying the rebuild option because the XUnitWrapperGenerator
project remained in the intermediate test folder and didn't get
actually rebuilt.

Thanks

Tomas
@ghost
Copy link

ghost commented Nov 24, 2021

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

I hit this problem locally when working on the test merging

  • I made a change in Jeremy's Roslyn generator logic adding a hard
    FailFast and the tests continued building normally even when
    specifying the rebuild option because the XUnitWrapperGenerator
    project remained in the intermediate test folder and didn't get
    actually rebuilt. As an additional tiny cleanup I have also replaced
    __TestWorkingDir with __TestBinDir in the Unix version of the script
    as they are always identical.

Thanks

Tomas

/cc @dotnet/runtime-infrastructure

Author: trylek
Assignees: -
Labels:

area-Infrastructure-coreclr

Milestone: -

@trylek trylek requested a review from jkoritzinsky November 24, 2021 00:48
if [[ -d "${__TestBinDir}" ]]; then
echo "Removing test build dir: ${__TestBinDir}"
rm -rf "${__TestBinDir}"
echo "Removing test intermediate dir: ${__TestIntermediatesDir}"
Copy link
Member

Choose a reason for hiding this comment

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

Does this need a preceding:

if [[ -d "${__TestIntermediatesDir}" ]]; then

? Or will rm -rf not complain if it doesn't exist? Of course, if __TestBinDir exists, it will essentially always exist anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks Bruce for pointing that out. I have locally verified that rm -rf has no trouble with non-existent folders so I just removed the pre-existing condition instead of introducing a new one.

I have double-checked locally that Unix has no problem whatsoever
with rm -rf on a non-existent directory. For this reason I have
removed the pre-existing conditional block instead of creating a new
one to cater for the intermediate folder.

Thanks

Tomas
@trylek
Copy link
Member Author

trylek commented Nov 24, 2021

As a side note, I'm not running outerloop or any other specialized pipelines as this change doesn't actually affect lab pipelines - the lab machines get cleaned up prior to each run - it is only useful in the context of local developer testing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants