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.

Sign up for GitHub

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

Jump to bottom

[Harness] Make CrashSnapshotReporter injectable and add unit tests #8129

Merged
merged 43 commits into from
Mar 20, 2020
Merged

[Harness] Make CrashSnapshotReporter injectable and add unit tests #8129

merged 43 commits into from
Mar 20, 2020

Conversation

premun
Copy link
Contributor

@premun premun commented Mar 17, 2020

Another dependency of AppRunner that will need to be refactored out to make AppRunner testable

Premek Vysoky added 30 commits March 13, 2020 11:27
Move AppInstallMonitorLog
a9c49ab
Move Helpers.GenerateStableGuid
ba7f37f
Move DirectoryUtilities
74d7816
Move DirectoryUtilities.RootDirectory
61646f1
Add CrashSnapshot ctor
88acdea
refactor CrashReportSnapshot
140a0cf
Add WrenchLog
9f20fd2
Move stuff out of Harness
d378104
Merge remote-tracking branch 'origin/master' into prvysoky/reafctor-u…
2b5c649 
…tils

Conflicts:
	tests/xharness/Harness.cs
	tests/xharness/Program.cs
Merge remote-tracking branch 'origin/master' into prvysoky/reafctor-u…
4fe4daa 
…tils
Remove RootDirectory
c38b390
Revert RootDirectory
9143073
Merge remote-tracking branch 'origin/master' into prvysoky/reafctor-u…
c9dad7a 
…tils

Conflicts:
	tests/xharness/AppRunner.cs
	tests/xharness/CrashReportSnapshot.cs
	tests/xharness/Harness.cs
	tests/xharness/xharness.csproj
Merge remote-tracking branch 'origin/master' into prvysoky/reafctor-u…
ec1c35e 
…tils
Merge remote-tracking branch 'origin/master' into prvysoky/reafctor-u…
27c873f 
…tils

Conflicts:
	tests/xharness/BCLTestImporter/BCLTestImportTargetFactory.cs
	tests/xharness/BCLTestImporter/BCLTestProjectGenerator.cs
	tests/xharness/xharness.csproj
Merge remote-tracking branch 'origin/master' into prvysoky/reafctor-u…
929fc60 
…tils
Remove Initialize() calls
fbcb828
Make CrashReportSnapshot injectable
3435023
Remove Harness dependency from CrashReportSnapshot
b3a7db5
Add device snapshot tests
a6298c0
Trim down IHarness and make properties of Harness readonly
735fadf
Merge remote-tracking branch 'premun/prvysoky/reafctor-utils' into pr…
2311882 
…vysoky/snapshots
Memoize xcoderoot
eabd707
Use MlaunchArguments
b68a323
Add MlaunchArguments docs
c407f39
Add CrashReportSnapshot docs
4185aec
Rename method
8329a72
Rename class
67b0093
Refactor CrashSnapshotReporter
19dff9a
Merge remote-tracking branch 'origin/master' into prvysoky/snapshots
b064cec 
Conflicts:
	tests/xharness/AppRunner.cs
	tests/xharness/CrashReportSnapshot.cs
	tests/xharness/Harness.cs
	tests/xharness/Jenkins/TestTasks/MacExecuteTask.cs
	tests/xharness/Jenkins/TestTasks/RunXtroTask.cs
mandel-macaque
mandel-macaque requested changes Mar 17, 2020
View reviewed changes
Copy link
Member

@mandel-macaque mandel-macaque left a comment

Choose a reason for hiding this comment

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

Minor comments to be fixed.

tests/xharness/CrashSnapshotReporter.cs Outdated Show resolved Hide resolved
tests/xharness/Execution/ProcessManager.cs Outdated Show resolved Hide resolved
tests/xharness/Execution/ProcessManager.cs Outdated Show resolved Hide resolved
rolfbjarne
rolfbjarne requested changes Mar 18, 2020
View reviewed changes
tests/xharness/AppRunner.cs Outdated Show resolved Hide resolved
@monojenkins
Copy link
Collaborator

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

@monojenkins
Copy link
Collaborator

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

@monojenkins
Copy link
Collaborator

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

@monojenkins
Copy link
Collaborator

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

Test results

2 tests failed, 85 tests passed.

Failed tests

  • xammac tests/Mac Modern/Debug: BuildFailure
  • xammac tests/Mac Modern/Release: BuildFailure

Merge remote-tracking branch 'origin/master' into prvysoky/snapshots
5630743 
Conflicts:
	tests/xharness/AppRunner.cs
	tests/xharness/Harness.cs
	tests/xharness/Jenkins/TestTasks/RunDeviceTask.cs
	tests/xharness/Jenkins/TestTasks/RunSimulatorTask.cs
	tests/xharness/Xharness.Tests/Tests/AppRunnerTests.cs
@monojenkins
Copy link
Collaborator

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

Test results

2 tests failed, 85 tests passed.

Failed tests

  • xammac tests/Mac Modern/Release: Failed (Test run failed.)
  • monotouch-test/tvOS - simulator/Debug (LinkSdk): Failed

@premun
Copy link
Contributor Author

premun commented Mar 19, 2020

@rolfbjarne tested that crash logs propagate back:
image

We can merge this (I think the failed tests are known issues)

mandel-macaque
mandel-macaque requested changes Mar 19, 2020
View reviewed changes
Copy link
Member

@mandel-macaque mandel-macaque left a comment

Choose a reason for hiding this comment

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

Small comments to address.

tests/xharness/CrashSnapshotReporter.cs Outdated Show resolved Hide resolved
tests/xharness/Execution/ProcessManager.cs Outdated Show resolved Hide resolved
tests/xharness/Execution/ProcessManager.cs Show resolved Hide resolved
tests/xharness/Xharness.Tests/Tests/CrashReportSnapshotTests.cs Show resolved Hide resolved
@monojenkins
Copy link
Collaborator

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

Test results

4 tests failed, 83 tests passed.

Failed tests

  • xammac tests/Mac Modern/Debug: BuildFailure
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (static registrar): Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Release (all optimizations): Failed
  • monotouch-test/tvOS - simulator/Debug: Failed

Merge remote-tracking branch 'origin/master' into prvysoky/snapshots
febde28 
Conflicts:
	tests/xharness/AppRunner.cs
	tests/xharness/Jenkins/Jenkins.cs
@monojenkins
Copy link
Collaborator

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

Test results

3 tests failed, 84 tests passed.

Failed tests

  • xammac tests/Mac Modern/Debug: BuildFailure
  • xammac tests/Mac Modern/Release: BuildFailure
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (LinkSdk): Failed

Merge remote-tracking branch 'origin/master' into prvysoky/snapshots
1032156 
Conflicts:
	tests/xharness/AppRunner.cs
	tests/xharness/Jenkins/TestTasks/RunTestTask.cs
@monojenkins
Copy link
Collaborator

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
ℹ️ Generator Diff (please review changes)
🔥 Test run failed 🔥

Test results

1 tests failed, 86 tests passed.

Failed tests

  • xammac tests/Mac Modern/Release: BuildFailure

@rolfbjarne
Copy link
Member

Test failure is unrelated: https://github.com/xamarin/maccore/issues/2177

@rolfbjarne rolfbjarne merged commit ec420e6 into xamarin:master Mar 20, 2020
@premun premun deleted the prvysoky/snapshots branch March 20, 2020 11:00
@rolfbjarne rolfbjarne added the not-notes-worthy Ignore for release notes label Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mandel-macaque mandel-macaque mandel-macaque approved these changes

@rolfbjarne rolfbjarne rolfbjarne approved these changes

Assignees
No one assigned
Labels
not-notes-worthy Ignore for release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@premun @monojenkins @rolfbjarne @xamarin-release-manager @mandel-macaque