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

Split out Smoke Tests and Unit Tests into a separate Jobs #3617

Merged
9 commits merged into from
Dec 15, 2020

Conversation

michael-hawker
Copy link
Member

@michael-hawker michael-hawker commented Dec 10, 2020

Fixes #3560

This PR does the following:

PR Type

  • Build or CI related changes

What is the current behavior?

Everything is done in one big job.

What is the new behavior?

We have more granularity on our pipeline

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Pull Request has been submitted to the documentation repository instructions. Link:
  • Sample in sample app has been added / updated (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

Other information

@ghost
Copy link

ghost commented Dec 10, 2020

Thanks michael-hawker for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@michael-hawker
Copy link
Member Author

Woot!

image

@azchohfi
Copy link
Contributor

Different jobs might (will) be executed on different machines on ADO Hosted environments, so to do this, you would need to make the artifacts from the build system available to the tests.
The smoke tests would be easier, since they use PackageReferences and are very disconnected from the build itself, so they only need the nuget packages in the correct output, but the unit tests have a strong ProjectReference to the main toolkit projects, so building it in a separate job would rebuild every project that the unit test references. Btw, the Unit tests are already built during the build stage that we have right now, but they are not executed.

@michael-hawker
Copy link
Member Author

When I merge the UI Tests, I should have a split condition to include package references for TAEF but keep the Project Reference for MSTest, this means we could split it out as a separate job to run in parallel.

@michael-hawker
Copy link
Member Author

Alright, cleaned up the YAML edits, think this should be good to run the Smoke Tests as a separate Job now. 🤞 Will check in the morning and look at splitting out the UI Integration Tests as another Job (as they're currently bundled together under the Unit Tests) as well doing the file measuring for the Smoke Tests, have some insights on that one. I don't think it should be too hard. 🤞

Makes it easier to see output from only UI tests without knowing a scroll point in the log
…and package sizes

Provides a more human readible way to understand our package sizes and dependencies against a baseline blank application
See more info in CommunityToolkit#3600 for potential improvements
@michael-hawker michael-hawker marked this pull request as ready for review December 12, 2020 00:03
@michael-hawker
Copy link
Member Author

Still some improvements we could do for #3600, but this gives us at least a starting point, so we can look to merge this in first and get back to improving things more later after 7.0.

For now, this should give us a good human-readable breakdown of both dependencies and the footprint size on an application.

Definitely some interesting finds... FYI @RosarioPulella

@michael-hawker
Copy link
Member Author

Aww, it doesn't colorize in Azure (seeing if I can get https://developercommunity.visualstudio.com/content/problem/440605/write-host-foreground-color-with-powershell-task-i.html re-opened), but worked like a charm still!

This is ready for review for the first version of this.

Copy link
Contributor

@Rosuavio Rosuavio left a comment

Choose a reason for hiding this comment

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

Other than some superfluous white space, Looks good!

azure-pipelines.yml Outdated Show resolved Hide resolved
azure-pipelines.yml Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Dec 15, 2020

Hello @michael-hawker!

Because this pull request has the auto merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@michael-hawker
Copy link
Member Author

@azchohfi this is all set to go for now. Look good to you?

@ghost ghost merged commit 3790024 into CommunityToolkit:master Dec 15, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Build] Create 2nd Stage/Job for Smoke Tests
3 participants