-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Preserve logs from retries as they have data! #19341
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,14 +5,50 @@ public interface IApp : IDisposable | |
IConfig Config { get; } | ||
IUIElementQueryable Query { get; } | ||
ApplicationState AppState { get; } | ||
|
||
IUIElement FindElement(string id); | ||
IUIElement FindElement(IQuery query); | ||
IReadOnlyCollection<IUIElement> FindElements(string id); | ||
IReadOnlyCollection<IUIElement> FindElements(IQuery query); | ||
string ElementTree { get; } | ||
|
||
ICommandExecution CommandExecutor { get; } | ||
void Click(float x, float y); | ||
} | ||
|
||
public interface IScreenshotSupportedApp : IApp | ||
{ | ||
FileInfo Screenshot(string fileName); | ||
byte[] Screenshot(); | ||
string ElementTree { get; } | ||
} | ||
Comment on lines
+18
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "optional APIs" should not be forced on the core testing |
||
|
||
public static class AppExtensions | ||
{ | ||
public static void Click(this IApp app, float x, float y) | ||
{ | ||
app.CommandExecutor.Execute("click", new Dictionary<string, object>() | ||
{ | ||
{ "x", x }, | ||
{ "y", y } | ||
}); | ||
} | ||
|
||
|
||
internal static T As<T>(this IApp app) | ||
where T : IApp | ||
{ | ||
if (app is not T derivedApp) | ||
throw new NotImplementedException($"The app '{app}' does not implement '{typeof(T).FullName}'."); | ||
|
||
return derivedApp; | ||
} | ||
} | ||
|
||
public static class ScreenshotSupportedAppExtensions | ||
{ | ||
public static FileInfo Screenshot(this IApp app, string fileName) => | ||
app.As<IScreenshotSupportedApp>().Screenshot(fileName); | ||
|
||
public static byte[] Screenshot(this IApp app) => | ||
app.As<IScreenshotSupportedApp>().Screenshot(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
using System.Reflection; | ||
using System.Runtime.CompilerServices; | ||
using NUnit.Framework; | ||
using NUnit.Framework.Interfaces; | ||
using UITest.Core; | ||
|
@@ -55,21 +56,21 @@ protected virtual void FixtureTeardown() | |
[TearDown] | ||
public void UITestBaseTearDown() | ||
{ | ||
|
||
if (App.AppState == ApplicationState.Not_Running) | ||
if (App.AppState == ApplicationState.NotRunning) | ||
{ | ||
// Assert.Fail will immediately exit the test which is desirable as the app is not | ||
// running anymore so we don't want to log diagnostic data as there is nothing to collect from | ||
Reset(); | ||
FixtureSetup(); | ||
|
||
// Assert.Fail will immediately exit the test which is desirable as the app is not | ||
// running anymore so we can't capture any UI structures or any screenshots | ||
Assert.Fail("The app was expected to be running still, investigate as possible crash"); | ||
} | ||
|
||
var testOutcome = TestContext.CurrentContext.Result.Outcome; | ||
if (testOutcome == ResultState.Error || | ||
testOutcome == ResultState.Failure) | ||
{ | ||
SaveDiagnosticLogs("UITestBaseTearDown"); | ||
SaveUIDiagnosticInfo(); | ||
} | ||
} | ||
|
||
|
@@ -84,7 +85,7 @@ public void OneTimeSetup() | |
} | ||
catch | ||
{ | ||
SaveDiagnosticLogs("FixtureSetup"); | ||
SaveUIDiagnosticInfo(); | ||
throw; | ||
} | ||
} | ||
|
@@ -98,36 +99,53 @@ public void OneTimeTearDown() | |
if (outcome.Status == ResultState.SetUpFailure.Status && | ||
outcome.Site == ResultState.SetUpFailure.Site) | ||
{ | ||
SaveDiagnosticLogs("OneTimeTearDown"); | ||
SaveUIDiagnosticInfo(); | ||
} | ||
|
||
FixtureTeardown(); | ||
} | ||
|
||
void SaveDiagnosticLogs(string? note = null) | ||
void SaveUIDiagnosticInfo([CallerMemberName] string? note = null) | ||
{ | ||
var screenshotPath = GetGeneratedFilePath("ScreenShot.png", note); | ||
if (screenshotPath is not null) | ||
{ | ||
_ = App.Screenshot(screenshotPath); | ||
|
||
AddTestAttachment(screenshotPath, Path.GetFileName(screenshotPath)); | ||
} | ||
|
||
var pageSourcePath = GetGeneratedFilePath("PageSource.txt", note); | ||
if (pageSourcePath is not null) | ||
{ | ||
File.WriteAllText(pageSourcePath, App.ElementTree); | ||
|
||
AddTestAttachment(pageSourcePath, Path.GetFileName(pageSourcePath)); | ||
} | ||
} | ||
|
||
string? GetGeneratedFilePath(string filename, string? note = null) | ||
{ | ||
Comment on lines
+127
to
+128
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This new method makes sure to get a new filename per run so that all logs are preserved. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But can we have a way to pass the guid from the cake? so we can map to what is on ci ui display name? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure, there might be multiple guids per run. If the test run crashes or appium somehow needs a restart, then we may have 2 items per single cake run. |
||
// App could be null if UITestContext was not able to connect to the test process (e.g. port already in use etc...) | ||
if (UITestContext is null) | ||
return null; | ||
|
||
if (string.IsNullOrEmpty(note)) | ||
note = "-"; | ||
else | ||
note = $"-{note}-"; | ||
|
||
var logDir = (Path.GetDirectoryName(Environment.GetEnvironmentVariable("APPIUM_LOG_FILE")) ?? Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location))!; | ||
filename = $"{Path.GetFileNameWithoutExtension(filename)}-{Guid.NewGuid().ToString("N")}{Path.GetExtension(filename)}"; | ||
|
||
// App could be null if UITestContext was not able to connect to the test process (e.g. port already in use etc...) | ||
if (UITestContext is not null) | ||
{ | ||
string name = TestContext.CurrentContext.Test.MethodName ?? TestContext.CurrentContext.Test.Name; | ||
var logDir = | ||
Path.GetDirectoryName(Environment.GetEnvironmentVariable("APPIUM_LOG_FILE") ?? | ||
Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location))!; | ||
|
||
var screenshotPath = Path.Combine(logDir, $"{name}-{_testDevice}{note}ScreenShot"); | ||
_ = App.Screenshot(screenshotPath); | ||
// App.Screenshot appends a ".png" extension always, so include that here | ||
var screenshotPathWithExtension = screenshotPath + ".png"; | ||
AddTestAttachment(screenshotPathWithExtension, Path.GetFileName(screenshotPathWithExtension)); | ||
var name = | ||
TestContext.CurrentContext.Test.MethodName ?? | ||
TestContext.CurrentContext.Test.Name; | ||
|
||
var pageSourcePath = Path.Combine(logDir, $"{name}-{_testDevice}{note}PageSource.txt"); | ||
File.WriteAllText(pageSourcePath, App.ElementTree); | ||
AddTestAttachment(pageSourcePath, Path.GetFileName(pageSourcePath)); | ||
} | ||
return Path.Combine(logDir, $"{name}-{_testDevice}{note}{filename}"); | ||
} | ||
|
||
void AddTestAttachment(string filePath, string? description = null) | ||
|
@@ -136,17 +154,10 @@ void AddTestAttachment(string filePath, string? description = null) | |
{ | ||
TestContext.AddTestAttachment(filePath, description); | ||
} | ||
catch (FileNotFoundException e) | ||
catch (FileNotFoundException e) when (e.Message == "Test attachment file path could not be found.") | ||
{ | ||
// Add the file path to better troubleshoot when these errors occur | ||
if (e.Message == "Test attachment file path could not be found.") | ||
{ | ||
throw new FileNotFoundException($"{e.Message}: {filePath}"); | ||
} | ||
else | ||
{ | ||
throw; | ||
} | ||
throw new FileNotFoundException($"Test attachment file path could not be found: '{filePath}' {description}", e); | ||
} | ||
} | ||
} | ||
|
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.
Don't force the
.png
here (even though it is a png) since this makes the other part of the code hard to reuse.