Skip to content

Commit 6a03614

Browse files
authored
Merge pull request #361 from csf-dev/356-unwanted-log-collection
Resolve #356 fix unwanted log collection
2 parents a6d82ad + c54e372 commit 6a03614

46 files changed

Lines changed: 375 additions & 177 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CSF.Screenplay.Selenium/Actions/OpenUrl.cs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
using System;
22
using System.Threading;
33
using System.Threading.Tasks;
4-
using OpenQA.Selenium;
5-
using static CSF.Screenplay.Selenium.PerformableBuilder;
64

75
namespace CSF.Screenplay.Selenium.Actions
86
{
@@ -11,10 +9,10 @@ namespace CSF.Screenplay.Selenium.Actions
119
/// </summary>
1210
/// <remarks>
1311
/// <para>
14-
/// Use this action via the builder method <see cref="PerformableBuilder.OpenTheUrl"/>.
12+
/// Use this action via the builder method <see cref="PerformableBuilder.NavigateTo"/>.
1513
/// The <c>OpenTheUrl</c> builder method does not have a one-to-one relationship with this action, though.
1614
/// The builder method actually returns a <xref href="TaskGlossaryItem?text=Screenplay+task"/> named
17-
/// <see cref="Tasks.OpenUrlRespectingBase"/>. The purpose of that task is to prepend a base URL to
15+
/// <see cref="Tasks.NavigateToUrl"/>. The purpose of that task is to prepend a base URL to
1816
/// URLs which are relative. This action is capable only of navigating to absolute URLs, and it will
1917
/// raise an exception if the URL is not absolute.
2018
/// </para>
@@ -42,22 +40,20 @@ namespace CSF.Screenplay.Selenium.Actions
4240
/// }
4341
/// </code>
4442
/// </example>
45-
/// <seealso cref="Tasks.OpenUrlRespectingBase"/>
46-
/// <seealso cref="PerformableBuilder.OpenTheUrl(NamedUri)"/>
43+
/// <seealso cref="Tasks.NavigateToUrl"/>
44+
/// <seealso cref="PerformableBuilder.NavigateTo(NamedUri)"/>
4745
public class OpenUrl : IPerformable, ICanReport
4846
{
4947
readonly NamedUri uri;
5048

5149
/// <inheritdoc/>
52-
public async ValueTask PerformAsAsync(ICanPerform actor, CancellationToken cancellationToken = default)
50+
public ValueTask PerformAsAsync(ICanPerform actor, CancellationToken cancellationToken = default)
5351
{
5452
var ability = actor.GetAbility<BrowseTheWeb>();
5553
if(!uri.Uri.IsAbsoluteUri)
5654
throw new InvalidOperationException($"The URL to open must be absolute; have you forgotten to grant {actor} the {nameof(UseABaseUri)} ability?");
5755
ability.WebDriver.Url = uri.Uri.ToString();
58-
59-
if(ability.ShouldCollectLogs && ability.WebDriver.HasQuirk(BrowserQuirks.CanGetLogsWithJavascriptWorkaround))
60-
await actor.PerformAsync(BeginCollectingLogsWithJavaScript(), cancellationToken);
56+
return default;
6157
}
6258

6359
/// <inheritdoc/>

CSF.Screenplay.Selenium/NamedUri.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace CSF.Screenplay.Selenium
99
/// <para>
1010
/// This model allows for the association of human-readable names with URIs. This is most useful when making use
1111
/// of Screenplay reports, where the human-readable name can be used to describe the URI in a more user-friendly way.
12-
/// Named Uris are used with the <see cref="Actions.OpenUrl"/> and <see cref="Tasks.OpenUrlRespectingBase"/> performables
12+
/// Named Uris are used with the <see cref="Actions.OpenUrl"/> and <see cref="Tasks.NavigateToUrl"/> performables
1313
/// to facilitate direct web browser navigation.
1414
/// </para>
1515
/// <para>
@@ -30,7 +30,7 @@ namespace CSF.Screenplay.Selenium
3030
/// </list>
3131
/// </remarks>
3232
/// <seealso cref="Actions.OpenUrl"/>
33-
/// <seealso cref="Tasks.OpenUrlRespectingBase"/>
33+
/// <seealso cref="Tasks.NavigateToUrl"/>
3434
/// <seealso cref="UseABaseUri"/>
3535
public sealed class NamedUri : IHasName
3636
{

CSF.Screenplay.Selenium/PerformableBuilder.general.cs

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,42 @@ namespace CSF.Screenplay.Selenium
1919
public static partial class PerformableBuilder
2020
{
2121
/// <summary>
22-
/// Gets a performable action which opens a URL.
22+
/// Gets a performable task which navigates to a URL.
2323
/// </summary>
2424
/// <remarks>
2525
/// <para>
26+
/// Consumers should prefer this task over the action exposed by <see cref="OpenTheUrl(NamedUri)"/>. This task takes care of some
27+
/// boilerplate concerns such as rebasing relative URLs onto a base URL, indicated by <see cref="UseABaseUri"/>.
28+
/// It also begins collecting logs using a JavaScript workaround if appropriate. Use this task to take care of those things
29+
/// automatically.
30+
/// </para>
31+
/// <para>
2632
/// If the specified Uri is a relative Uri, then this task will use the actor's <see cref="UseABaseUri"/> ability (if present)
2733
/// to transform the relative Uri into an absolute one. The specified Uri will be used directly if it is already absolute.
2834
/// </para>
35+
/// <para>
36+
/// Additionally, if it is appropriate to do so, after the web browser arrives at the new page, JavaScript will be used to
37+
/// begin collecting browser console logs from the page. See <see cref="BeginCollectingLogsWithJavaScriptIfApplicable"/> for
38+
/// more information.
39+
/// </para>
2940
/// </remarks>
3041
/// <param name="uri">The uri at which to open the web browser.</param>
3142
/// <returns>A performable</returns>
32-
public static IPerformable OpenTheUrl(NamedUri uri) => new OpenUrlRespectingBase(uri);
43+
public static IPerformable NavigateTo(NamedUri uri) => new NavigateToUrl(uri);
44+
45+
/// <summary>
46+
/// Gets a performable action which directly opens a URL (and does nothing more).
47+
/// </summary>
48+
/// <remarks>
49+
/// <para>
50+
/// Consumers are generally recommended to prefer <see cref="NavigateTo(NamedUri)"/> over this action.
51+
/// This action only opens the web browser at the specified Url and does nothing more.
52+
/// It will throw an exception if the Url is not absolute.
53+
/// </para>
54+
/// </remarks>
55+
/// <param name="uri">The uri at which to open the web browser.</param>
56+
/// <returns>A performable</returns>
57+
public static IPerformable OpenTheUrl(NamedUri uri) => new OpenUrl(uri);
3358

3459
/// <summary>
3560
/// Gets a performable question which takes a screenshot of the current web page and returns it.
@@ -209,16 +234,34 @@ public static NamedWaitBuilder WaitUntil(IBuildsElementPredicates predicate)
209234
/// <returns>A performable question.</returns>
210235
public static GetWindowTitle ReadTheWindowTitle() => new GetWindowTitle();
211236

237+
/// <summary>
238+
/// Gets a performable task which begins collecting log information with a Javascript workaround, if the current WebDriver configuration
239+
/// supports it and requires it.
240+
/// </summary>
241+
/// <remarks>
242+
/// <para>
243+
/// It is usually not required to use this task directly.
244+
/// The recommended way to consume this action is to set <see cref="BrowseTheWeb.ShouldCollectLogs"/> to <see langword="true"/>
245+
/// via the ability's constructor. If the WebDriver has the quirk <see cref="BrowserQuirks.CanGetLogsWithJavascriptWorkaround"/>
246+
/// and does not have the quirk <see cref="BrowserQuirks.HasNativeLogsSupport"/> AND the browser is not a
247+
/// <see cref="OpenQA.Selenium.Remote.RemoteWebDriver"/> then
248+
/// this task will be executed automatically at appropriate times, such as immediately after <see cref="ClickAndWaitForDocumentReady"/>
249+
/// or <see cref="OpenUrl"/> is executed.
250+
/// </para>
251+
/// </remarks>
252+
/// <returns>A performable task</returns>
253+
public static BeginCollectingLogsWithJavaScriptIfApplicable BeginCollectingLogsWithJavaScriptIfApplicable()
254+
=> new BeginCollectingLogsWithJavaScriptIfApplicable();
255+
212256
/// <summary>
213257
/// Gets a performable action which begins collecting log information with a Javascript workaround.
214258
/// </summary>
215259
/// <remarks>
216260
/// <para>
217261
/// It is usually not required to use this action directly.
218262
/// The recommended way to consume this action is to set <see cref="BrowseTheWeb.ShouldCollectLogs"/> to <see langword="true"/>
219-
/// via the ability's constructor. If the WebDriver has the quirk <see cref="BrowserQuirks.CanGetLogsWithJavascriptWorkaround"/> then
220-
/// this action will be executed automatically at appropriate times, such as immediately after <see cref="ClickAndWaitForDocumentReady"/>
221-
/// or <see cref="OpenUrl"/> is executed.
263+
/// via the ability's constructor. When the Web Driver navigates to a new page, <see cref="BeginCollectingLogsWithJavaScriptIfApplicable"/>
264+
/// will be used, which will conditionally trigger this action if applicable.
222265
/// </para>
223266
/// </remarks>
224267
/// <returns>A performable action</returns>

CSF.Screenplay.Selenium/Questions/GetLogsWithJavaScript.cs

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,6 @@ namespace CSF.Screenplay.Selenium.Questions
3535
/// <see cref="OpenQA.Selenium.DriverOptions.SetLoggingPreference(string, OpenQA.Selenium.LogLevel)"/>.
3636
/// Consumers will need to manually filter for messages which they care about, based upon the log level.
3737
/// </para>
38-
/// <para>
39-
/// Also note that this technique is not viable with Remote WebDrivers. It has been tested and found to be unreliable.
40-
/// This Question will throw an exception if the underlying WebDriver is an instance of
41-
/// <see cref="OpenQA.Selenium.Remote.RemoteWebDriver"/>. Instead of using this technique, use the remote web driver's
42-
/// own built-in technique to access console logs. For example, some providers offer a separate non-WebDriver-based API
43-
/// to read the logs. Accessing these is outside the scope of this class.
44-
/// </para>
4538
/// </remarks>
4639
public class GetLogsWithJavaScript : IPerformableWithResult<IReadOnlyList<BrowserLog>>, ICanReport
4740
{
@@ -55,8 +48,6 @@ public async ValueTask<IReadOnlyList<BrowserLog>> PerformAsAsync(ICanPerform act
5548
var driver = actor.GetAbility<BrowseTheWeb>();
5649
if(!driver.WebDriver.HasQuirk(BrowserQuirks.CanGetLogsWithJavascriptWorkaround))
5750
throw new NotSupportedException("The WebDriver must have support for retrieving logs using a Javascript workaround.");
58-
if(driver.WebDriver.Unproxy() is OpenQA.Selenium.Remote.RemoteWebDriver)
59-
throw new NotSupportedException("Getting logs via JavaScript is not supported for Remote Web Drivers, see the documentation for this class for more information");
6051

6152
var result = await actor.PerformAsync(PerformableBuilder.ExecuteAScript(Scripts.GetLogs), cancellationToken);
6253
return result
Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,45 @@
11
(function() {
2-
if(globalThis['__csfScreenplayLogs']) return;
2+
const
3+
storageKey = '__csfScreenplayLogs',
4+
orig = {};
5+
globalThis.sessionStorage.setItem(storageKey, '[]');
36

4-
globalThis['__csfScreenplayLogs'] = {originalFunctions:{},messages:[]};
5-
const logs = globalThis['__csfScreenplayLogs'];
6-
7-
logs.originalFunctions.log = globalThis.console.log;
8-
logs.originalFunctions.info = globalThis.console.info;
9-
logs.originalFunctions.warn = globalThis.console.warn;
10-
logs.originalFunctions.error = globalThis.console.error;
11-
logs.originalFunctions.debug = globalThis.console.debug;
12-
logs.originalFunctions.clear = globalThis.console.clear;
7+
orig.log = globalThis.console.log;
8+
orig.info = globalThis.console.info;
9+
orig.warn = globalThis.console.warn;
10+
orig.error = globalThis.console.error;
11+
orig.debug = globalThis.console.debug;
12+
orig.clear = globalThis.console.clear;
1313

1414
globalThis.console.log = function(...args) {
1515
captureMessage('Info', args);
16-
logs.originalFunctions.log(...args);
16+
orig.log(...args);
1717
}
1818
globalThis.console.info = function(...args) {
1919
captureMessage('Info', args);
20-
logs.originalFunctions.info(...args);
20+
orig.info(...args);
2121
}
2222
globalThis.console.warn = function(...args) {
2323
captureMessage('Warning', args);
24-
logs.originalFunctions.warn(...args);
24+
orig.warn(...args);
2525
}
2626
globalThis.console.error = function(...args) {
2727
captureMessage('Severe', args);
28-
logs.originalFunctions.error(...args);
28+
orig.error(...args);
2929
}
3030
globalThis.console.debug = function(...args) {
3131
captureMessage('Debug', args);
32-
logs.originalFunctions.debug(...args);
32+
orig.debug(...args);
3333
}
3434
globalThis.console.clear = function() {
3535
for(const message of logs.messages)
3636
message.Consumed = true;
37-
logs.originalFunctions.clear();
37+
orig.clear();
3838
}
3939

4040
function captureMessage(level, args) {
41-
logs.messages.push({Level:level,Message:args,Timestamp:new Date()});
41+
const logs = JSON.parse(globalThis.sessionStorage.getItem(storageKey));
42+
logs.push({Level:level,Message:args,Timestamp:new Date()});
43+
globalThis.sessionStorage.setItem(storageKey, JSON.stringify(logs));
4244
}
4345
})();
Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
return function() {
2-
if(!globalThis['__csfScreenplayLogs']) throw new Error('The CaptureLogs script must have been executed on the current page before GetLogs may be used');
2+
const
3+
storageKey = '__csfScreenplayLogs',
4+
allLogsStr = globalThis.sessionStorage.getItem(storageKey);
35

4-
const logs = globalThis['__csfScreenplayLogs'].messages.filter(x => !x.Consumed);
5-
for(const log of logs)
6-
log.Consumed = true;
7-
return logs.map(x => ({
6+
if(!allLogsStr) throw new Error('The CaptureLogs script must have been executed on the current page before GetLogs may be used');
7+
8+
globalThis.sessionStorage.setItem(storageKey, '[]');
9+
return JSON.parse(allLogsStr).map(x => ({
810
Level: x.Level,
9-
Message: x.Message.toString(),
10-
Timestamp: x.Timestamp.toISOString()
11+
Message: JSON.stringify(x.Message),
12+
Timestamp: x.Timestamp
1113
}));
1214
}();
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
using System;
2+
using System.Threading;
3+
using System.Threading.Tasks;
4+
using OpenQA.Selenium;
5+
using OpenQA.Selenium.Remote;
6+
using static CSF.Screenplay.Selenium.PerformableBuilder;
7+
8+
namespace CSF.Screenplay.Selenium.Tasks
9+
{
10+
/// <summary>
11+
/// Determines whether the current actor needs to use a JS technique to collect web browser console logs.
12+
/// If they do, executes the action which begins collecting them.
13+
/// </summary>
14+
/// <remarks>
15+
/// <para>
16+
/// When used, this action should be executed as soon as possible after the current page has completed loading.
17+
/// Ideally, directly after <see cref="ClickAndWaitForDocumentReady"/> and <see cref="Actions.OpenUrl"/>.
18+
/// This Task decides whether it is appropriate to make use of <see cref="Actions.BeginCollectingLogsWithJavaScript"/>
19+
/// action, invoking it if appropriate.
20+
/// </para>
21+
/// <para>
22+
/// Note that this action/the script needs to be re-run after each traditional web page navigation/reload.
23+
/// However, due to the nature of SPAs, it <em>does not need to be re-run</em> following an SPA-style navigation.
24+
/// On supported browsers, there is no harm in re-running this script when it is not needed, except for the impact on
25+
/// performance (wasted network roundtrips).
26+
/// </para>
27+
/// <para>
28+
/// The logic used by this task is:
29+
/// </para>
30+
/// <list type="bullet">
31+
/// <item><description>If the actor's <see cref="BrowseTheWeb"/> ability has <see cref="BrowseTheWeb.ShouldCollectLogs"/>
32+
/// set to <see langword="false"/> then the JavaScript technique <em>will not</em> be used.</description></item>
33+
/// <item><description>If the actor's <see cref="BrowseTheWeb"/> ability has the quirk <see cref="BrowserQuirks.HasNativeLogsSupport"/>
34+
/// <em>and</em> the implementation of <see cref="IWebDriver"/> is not a <see cref="RemoteWebDriver"/> then the JavaScript
35+
/// technique <em>will not</em> be used.</description></item>
36+
/// <item><description>If the <see cref="IWebDriver"/> does not have the quirk <see cref="BrowserQuirks.CanGetLogsWithJavascriptWorkaround"/>
37+
/// then the JavaScript technique <em>will not</em> be used.</description></item>
38+
/// <item><description>Otherwise, <see cref="BrowseTheWeb.ShouldCollectLogs"/> is <see langword="true"/>, the <see cref="IWebDriver"/>
39+
/// has the quirk <see cref="BrowserQuirks.CanGetLogsWithJavascriptWorkaround"/> and does not have a better way to get the logs, then the
40+
/// JavaScript technique <em>will be used</em>.</description></item>
41+
/// </list>
42+
/// <para>
43+
/// Note that <em><see cref="RemoteWebDriver"/> instances cannot collect the logs natively</em>. This is because the WebDriver remote protocol
44+
/// does not support this functionality. Web Drivers which support it locally cannot do so remotely. See
45+
/// <see href="https://github.com/w3c/webdriver/issues/1428">this issue about logging being missing from the w3c WebDriver standard</see>
46+
/// and the signposted <see href="https://www.w3.org/2018/10/25-webdriver-minutes.html#item05">minutes from this meeting</see> for more info.
47+
/// </para>
48+
/// </remarks>
49+
public class BeginCollectingLogsWithJavaScriptIfApplicable : IPerformable, ICanReport
50+
{
51+
/// <inheritdoc/>
52+
public ReportFragment GetReportFragment(Actor actor, IFormatsReportFragment formatter)
53+
{
54+
var shouldPerform = ShouldCollectLogsWithJavaScript(actor);
55+
return formatter.Format("{Actor} chooses whether they should collect web browser console logs using JavaScript; {Outcome}",
56+
actor,
57+
shouldPerform ? "they should" : "they should not");
58+
59+
}
60+
61+
/// <inheritdoc/>
62+
public async ValueTask PerformAsAsync(ICanPerform actor, CancellationToken cancellationToken = default)
63+
{
64+
var shouldPerform = ShouldCollectLogsWithJavaScript(actor);
65+
if(!shouldPerform) return;
66+
67+
await actor.PerformAsync(BeginCollectingLogsWithJavaScript(), cancellationToken);
68+
}
69+
70+
/// <summary>
71+
/// Gets a value indicating whether the specified actor should use the JavaScript technique to get browser logs.
72+
/// </summary>
73+
/// <remarks>
74+
/// <para>
75+
/// See the remarks for this type for more information about that logic.
76+
/// </para>
77+
/// </remarks>
78+
/// <param name="actor">The actor</param>
79+
/// <returns><see langword="true"/> if the JavaScript technique for log collection is applicable; <see langword="false"/> if not.</returns>
80+
public static bool ShouldCollectLogsWithJavaScript(ICanPerform actor)
81+
{
82+
var browseTheWeb = actor.GetAbility<BrowseTheWeb>();
83+
return ShouldCollectLogsWithJavaScript(browseTheWeb);
84+
}
85+
86+
/// <summary>
87+
/// Gets a value indicating whether the specified browse the web ability should use the JavaScript technique to get browser logs.
88+
/// </summary>
89+
/// <remarks>
90+
/// <para>
91+
/// See the remarks for this type for more information about that logic.
92+
/// </para>
93+
/// </remarks>
94+
/// <param name="browseTheWeb">The ability</param>
95+
/// <returns><see langword="true"/> if the JavaScript technique for log collection is applicable; <see langword="false"/> if not.</returns>
96+
public static bool ShouldCollectLogsWithJavaScript(BrowseTheWeb browseTheWeb)
97+
{
98+
if(!browseTheWeb.ShouldCollectLogs) return false;
99+
100+
return browseTheWeb.WebDriver.HasQuirk(BrowserQuirks.CanGetLogsWithJavascriptWorkaround)
101+
&& (!browseTheWeb.WebDriver.HasQuirk(BrowserQuirks.HasNativeLogsSupport)
102+
|| browseTheWeb.WebDriver.Unproxy() is RemoteWebDriver);
103+
}
104+
}
105+
}

0 commit comments

Comments
 (0)