Run tests for dnup branch across all 3 OS in PRs
#51372
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
im not sure if I really like this - chose a fortress to represent 'guarding' the code by running tests.... is it really helpful? It might be if / when we have other test steps to differentiate and find the dnup test checks.
there is no concept of a global\ mutex prefix on unix which could be incorrect. On windows, this makes it so the mutex is not for the local user session but instead across all users. I'm not sure if this is needed but since we edit the system path, it might be. I will have to think about whether there will be issues with a higher-level admin user that has held onto a mutex and got stuck, and whether a lower level user could undo that.
10 seconds may fail - concurrently the tests could take a while. lets not fail the test because other tests are running.
OpenExisting may run into challenges with security policies changing - let's not try to navigate that when we can track it ourselves.
this would likely cause concurrency issues in tests if we just rely on the environment variable.
tests total runtime is 200 s on my machine - let's see if this helps. this would ideally be configurable because most users probably won't run multiple installs, though also they may have a much slower machine where this would cause concurrent requests to fail.
may refactor this later...
we probably shouldnt hard code debug amongst other things. Im sure theres a better approach but just want to get it green first.
This implements validation of installs using hostfxr apis to ensure the install actually works and not just that the manifest is tracking the installs correctly in e2e tests. It also adds a test to show that we can do multiple installs in the same directory without failing. It also improves the existing test logic to not assume a hard-coded debug value for the dnup process.
This reverts commit 0fc7172.
|
Just going to merge this so other code isn't blocked - will fix the hardcoded debug expectations / whitespace later, then add further validation and tests in another PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We want to ensure that nothing breaks when we refactor code and that we have proper test driven development. #51349 started this but we needed to have the pipeline file pushed into the branch to make the pipeline. This actually fixes the pipeline and makes it work.