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

We're currently copying the test assets to the publish directory #43146

Merged
merged 2 commits into from
Sep 3, 2024

Conversation

marcpopMSFT
Copy link
Member

for every single test project which is duplicating a lot of files in our PR builds.

This is also leading to some .tmp file missing copy issues from the parallel builds I'm not sure why these files are specifically included so let's try removing them and see what fails.

…very single test project which is duplicating a lot of files in our PR builds.

This is also leading to some .tmp file missing copy issues from the parallel builds
I'm not sure why these files are specifically included so let's try removing them and see what fails.
Copy link
Member

@MiYanni MiYanni left a comment

Choose a reason for hiding this comment

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

I have no earthly idea why this wasn't there before. When I dug into the code, it fell into that else path because of how it, I believe, uses environment variables to set certain values when running in Helix. I put a comment here on Will's PR to add Helix support when I was investing this stuff before. Anyway, if this PR works, then go for it.

@marcpopMSFT
Copy link
Member Author

I have no earthly idea why this wasn't there before. When I dug into the code, it fell into that else path because of how it, I believe, uses environment variables to set certain values when running in Helix. I put a comment here on Will's PR to add Helix support when I was investing this stuff before. Anyway, if this PR works, then go for it.

Yeah, I'm not sure why some changes didn't work before but this is passing now. Maybe something else changed in between. I want to test local test build before I merge this just in case (don't want to cause local dev tests run to fail just to fix helix runs).

And RE comment in Will's PR, I think the initial testexecutiondirectory set can just be deleted...may be worth trying in a separate PR just to clean it up.

@marcpopMSFT
Copy link
Member Author

Ran a bunch of tools tests locally and they passed so I think this is good for helix and local dev. I didn't test local helix but that config is already fairly challenging to get set up.

@marcpopMSFT marcpopMSFT merged commit 5ad8c9f into release/9.0.1xx Sep 3, 2024
31 checks passed
@marcpopMSFT marcpopMSFT deleted the marcpopMSFT-fixtmpissue branch September 3, 2024 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants