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

Azure Pipelines: Skip tests if build fails, run both tools even if other fails #1337

Merged
merged 3 commits into from
Oct 9, 2020
Merged

Conversation

sylveon
Copy link
Contributor

@sylveon sylveon commented Oct 2, 2020

Fixes #1266

This adds basic build dependencies to the CI config. I've only added some basic dependencies to keep it simple, since the rest should eventually cascade to a build failure.

@sylveon sylveon requested a review from a team as a code owner October 2, 2020 02:07
@BillyONeal
Copy link
Member

High 5 for getting the best PR number possible.

@sylveon
Copy link
Contributor Author

sylveon commented Oct 2, 2020

After making my other PR I just had to grab that number :P

Looks like steps don't support dependsOn, but I might be able to use condition instead, looking through the docs ATM.

@sylveon
Copy link
Contributor Author

sylveon commented Oct 2, 2020

Conditions in steps won't really work because you can't verify if a specific step failed or not.

One idea that I have (at least for the validation stage) is to split it in 3 jobs:

  • One that builds the tooling
  • One that runs clang-format
  • One that validates files

So that way, we can specify job dependencies normally. Same idea could be applied to the build & test jobs if each architecture is its own stage, rather than its own job.

@CaseyCarter CaseyCarter added the infrastructure Related to repository automation label Oct 2, 2020
@CaseyCarter
Copy link
Member

Looks like this is Work in Progress? Shout if I'm wrong, or when you push some changes and it's ready for review.

@sylveon
Copy link
Contributor Author

sylveon commented Oct 2, 2020

Yeah, but this is CI changes and IIRC draft PRs don't trigger CI, but since my changes are about CI...

@StephanTLavavej
Copy link
Member

Makes sense - thanks for investigating this, and feel free to continue pushing commits to trigger the CI. (This PR will ultimately save us lots of resources when builds fail!)

@SuperWig
Copy link
Contributor

SuperWig commented Oct 2, 2020

draft PRs don't trigger CI

Unless that changed recently it should.

@CaseyCarter
Copy link
Member

Yeah, but this is CI changes and IIRC draft PRs don't trigger CI, but since my changes are about CI...

That wasn't griping about making it a Draft PR - I can never remember if they trigger CI - just communicating my understanding of its status and choice of stage in https://github.com/microsoft/STL/projects/1. I wanted to ensure we're on the same page so you're not inadvertently left sitting here waiting for maintainer feedback.

@sylveon sylveon changed the title Azure Pipelines: Add basic build dependencies Azure Pipelines: Skip tests if build fails, run clang-format and validate in parallel Oct 3, 2020
@sylveon
Copy link
Contributor Author

sylveon commented Oct 3, 2020

@CaseyCarter I believe this is ready for review!

Here's an example of a failing build that skips tests: https://dev.azure.com/vclibs/STL/_build/results?buildId=5335&view=results

I also squashed all my commits because testing CI is p a i n

azure-pipelines.yml Outdated Show resolved Hide resolved
azure-devops/run-build.yml Show resolved Hide resolved
@sylveon sylveon changed the title Azure Pipelines: Skip tests if build fails, run clang-format and validate in parallel Azure Pipelines: Skip tests if build fails, run both tools even if other fails Oct 6, 2020
@cbezault
Copy link
Contributor

cbezault commented Oct 7, 2020

Sorry I made you throw away some of your work :(
LGTM now

@sylveon
Copy link
Contributor Author

sylveon commented Oct 7, 2020

Haha it's fine I often throw away my work myself.

Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Thanks for this significant infrastructure improvement - I think it'll improve the experience for contributors and decrease load on the VMSS which is awesome.

@CaseyCarter CaseyCarter self-assigned this Oct 8, 2020
@CaseyCarter
Copy link
Member

FYI, I'm about to merge this. If the CI stops working, I will likely snap my fingers Thanatos-style and destroy a significant portion of the universe.

@CaseyCarter CaseyCarter merged commit 8050cbe into microsoft:master Oct 9, 2020
@CaseyCarter
Copy link
Member

Thanks for the contribution! (I'm 100% behind anyone-but-me enjoying the "edit-submit-wait an hour for CI" loop!)

@CaseyCarter CaseyCarter removed their assignment Oct 9, 2020
CaseyCarter added a commit that referenced this pull request Oct 9, 2020
feature/spaceship: Merge CI changes to pick up #1337. This is a clean merge with no additional edits.
CaseyCarter added a commit that referenced this pull request Oct 9, 2020
feature/format: Merge CI update to pick up #1337. There was a minor merge conflict in `test/std/test.lst` due to the addition of tests on both branches.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Related to repository automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Azure Pipelines: When the build fails, we shouldn't run tests
6 participants