-
Notifications
You must be signed in to change notification settings - Fork 384
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
Azure pipelines migration #1267
Azure pipelines migration #1267
Conversation
… into CI_NotAppVeyorSpecific
My current suspicion is that the Ubuntu 18.04 problems are caused by the |
Hmmmm just confirmed it wasn't that and haven't been able to reproduce yet on my ubuntu 18.04 VM |
We should certainly be building the module to make sure it builds in CI, but not sure if you mean that there's another call to that build script elsewhere that outmodes it? |
# Conflicts: # Tests/Engine/CustomizedRule.tests.ps1
Close and re-open to see if things have changed and if the new Ubuntu failures on AppVeyor, happen here as well. |
Hmmm, so it is reproducing in AzDO CI as well. After spending a day trying to reproduce this, I stopped trying when @JamesWTruher and I didn't see it in AzDO (only AppVeyor). Since this seems to be causing an issue in AzDO now, I'll go back to trying to reproduce it. I didn't have much luck replicating the AzDO environment locally... |
Maybe just ignoring the failures similar to the AppVeyor ones would be the best? Interestingly there were new AppVeyor AppVeyor failures just before xmas in AppVeyor but they have disappeared now |
Frustrating. But yes, I don't think these failures should block anything, so we should ignore them for now. |
I'll open a PR to ignore them in AzDO |
I suggest to just add them in this PR here and use an environment variable that is only set in Az-Do |
Ok with you if I commit to this branch? |
Yes, please |
Getting this:
Going to open a PR instead |
See bergmeister#11 |
Exclude failing tests on AzDO/Ubuntu
Don't know why the push did not work, especially since I had the 'Allow edits from maintainers' checkbox ticked. Thanks for the PR of the PR. |
Great, the Ubuntu build fails now because somehow after bootstrap the .net core sdk, it seems to lose it on the PATH env variable. Will merge bootstrap and build tasks into one then |
… into AzurePipelinesMigration Merge using HEAD, which reverts the last commit... # Conflicts: # build.psm1
inputs: | ||
testRunner: NUnit | ||
testResultsFiles: 'TestResults.xml' | ||
condition: succeededOrFailed() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something that should be said is that although I set it to always run, but when the build or test fails, this task does not run and therefore test results are not uploaded. This seems to be a bug with the Azure DevOps YAML though. We should raise something but I don't think that's a blocking issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JamesWTruher FYI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR Summary
This is the minimum viable, lift and shift approach to have test running in Azure DevOps.
For better visibility in a PR I decided to create multiple build definitions instead of using multiple jobs in one build definition and another reason is that if one used multiple job in one build definition, one would not be able to see from which platform they came.
Good news: They pass on macOS but there are some test failures to be looked into for Ubuntu. I suggest to only validate against PS 5/6 on Windows and Mac and keep track of the Ubuntu failures until we have fixed them. Somehow there is also a bug in Azure DevOps where the task to upload test results it supposed to always run using
succeededOrFailed
but in practice it does not run if the previous task fails...After this I suggest we only keep the PS v4 test in AppVeyor
@rjmholt Can I ask you please to look into the test failures from your compatibility module please? It seems that those tests pass on Ubuntu 16 but not Ubuntu 18, see related PR #1262 where I tried to fix the Ubuntu builds in AppVeyor. Also: I was not sure if we still need the calls to
./PSCompatibilityCollector/build.ps1
in the build yaml script?PR Checklist
.cs
,.ps1
and.psm1
files have the correct copyright headerWIP:
to the beginning of the title and remove the prefix when the PR is ready.