-
Notifications
You must be signed in to change notification settings - Fork 2
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
FED-466: Fix console logs that require formatting #52
Changes from 6 commits
aa96e2f
c6fde52
8f7b9d4
19b2de9
5eeb3a0
6790c0d
800eeb3
2e3d986
cfc5f88
2b39903
c7beb86
80acd97
3990a38
b814c35
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 | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,72 @@ | ||||||
// This code was adapted to Dart from | ||||||
// https://github.com/nodejs/node-v0.x-archive/blob/master/lib/util.js | ||||||
// | ||||||
// Copyright Joyent, Inc. and other Node contributors. | ||||||
// | ||||||
// Permission is hereby granted, free of charge, to any person obtaining a | ||||||
// copy of this software and associated documentation files (the | ||||||
// "Software"), to deal in the Software without restriction, including | ||||||
// without limitation the rights to use, copy, modify, merge, publish, | ||||||
// distribute, sublicense, and/or sell copies of the Software, and to permit | ||||||
// persons to whom the Software is furnished to do so, subject to the | ||||||
// following conditions: | ||||||
// | ||||||
// The above copyright notice and this permission notice shall be included | ||||||
// in all copies or substantial portions of the Software. | ||||||
// | ||||||
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS | ||||||
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF | ||||||
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN | ||||||
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, | ||||||
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR | ||||||
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE | ||||||
// USE OR OTHER DEALINGS IN THE SOFTWARE. | ||||||
|
||||||
@JS() | ||||||
library console_log_formatter; | ||||||
|
||||||
import 'package:js/js.dart'; | ||||||
|
||||||
@JS('JSON.stringify') | ||||||
external String _jsonStringify(dynamic object); | ||||||
|
||||||
final _formatRegExp = RegExp('%[sdj%]'); | ||||||
|
||||||
/// A doc comment | ||||||
String format(dynamic f, List<dynamic> arguments) { | ||||||
if (f is! String) { | ||||||
return [f, ...arguments].join(' '); | ||||||
} | ||||||
|
||||||
var str = ''; | ||||||
if (f is String) { | ||||||
var i = 0; | ||||||
final len = arguments.length; | ||||||
str += f.replaceAllMapped(_formatRegExp, (m) { | ||||||
final x = m[0]; | ||||||
if (x == '%%') return '%'; | ||||||
if (i >= len) return x; | ||||||
switch (x) { | ||||||
case '%s': | ||||||
return arguments[i++].toString(); | ||||||
case '%d': | ||||||
return num.tryParse(arguments[i++].toString()).toString(); | ||||||
case '%j': | ||||||
try { | ||||||
return _jsonStringify([arguments[i++]]); | ||||||
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. Since this isn't callMethod, it shouldn't be wrapped in a list.
Suggested change
|
||||||
} catch (_) { | ||||||
return x; | ||||||
} | ||||||
break; | ||||||
default: | ||||||
return x; | ||||||
} | ||||||
}); | ||||||
|
||||||
if (i < len) { | ||||||
str += ' ${arguments.skip(i).join(' ')}'; | ||||||
} | ||||||
} | ||||||
|
||||||
return str; | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ | |
|
||
import 'dart:async'; | ||
import 'dart:html'; | ||
import 'dart:js_util'; | ||
|
||
import 'package:react/react.dart' as react; | ||
import 'package:react/react_client/react_interop.dart'; | ||
|
@@ -130,6 +131,52 @@ void main() { | |
expect(printCalls, ['foo', 'bar']); | ||
}); | ||
|
||
group('prints logs that use formatter syntax', () { | ||
test('with exact amount of args', () { | ||
// This also tests functionally that the print call occurs in the right zone, | ||
// which need to happen for them to be forwarded in the terminal in a test environment. | ||
// If it wasn't in the right zone, we wouldn't be able to record it, and neither would | ||
// the test package. | ||
final printCalls = recordPrintCalls(() { | ||
printConsoleLogs(() { | ||
callMethod(getProperty(window, 'console'), 'log', ['%s World Number %d! %j', 'Hello']); | ||
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. Looks like this is missing the jsified argument, the number, and the additional arg |
||
}); | ||
}); | ||
expect(printCalls, ['Hello World Number 5! {"doWeComeInPeace":false} additional']); | ||
}); | ||
|
||
test('with too many args', () { | ||
// This also tests functionally that the print call occurs in the right zone, | ||
// which need to happen for them to be forwarded in the terminal in a test environment. | ||
// If it wasn't in the right zone, we wouldn't be able to record it, and neither would | ||
// the test package. | ||
final printCalls = recordPrintCalls(() { | ||
printConsoleLogs(() { | ||
callMethod(getProperty(window, 'console'), 'log', [ | ||
'%s World Number %d! %j', | ||
'Hello', | ||
5, | ||
jsify({'doWeComeInPeace': false}) | ||
]); | ||
}); | ||
}); | ||
expect(printCalls, ['Hello World Number 5! {"doWeComeInPeace":false} additional']); | ||
}); | ||
|
||
test('without enough args', () { | ||
// This also tests functionally that the print call occurs in the right zone, | ||
// which need to happen for them to be forwarded in the terminal in a test environment. | ||
// If it wasn't in the right zone, we wouldn't be able to record it, and neither would | ||
// the test package. | ||
final printCalls = recordPrintCalls(() { | ||
printConsoleLogs(() { | ||
callMethod(getProperty(window, 'console'), 'log', ['%s World Number %d! %j', 'Hello']); | ||
}); | ||
}); | ||
expect(printCalls, ['Hello World Number 5! {"doWeComeInPeace":false} additional']); | ||
}); | ||
}); | ||
|
||
test('prints even if the function throws partway through', () { | ||
final printCalls = recordPrintCalls(() { | ||
expect(() { | ||
|
@@ -183,10 +230,7 @@ void main() { | |
]; | ||
|
||
if (runtimeSupportsPropTypeWarnings()) { | ||
expect( | ||
logs, | ||
unorderedEquals( | ||
[...expectedLogs, contains('shouldNeverBeNull is necessary.'), contains('⚠️ Warning:')])); | ||
expect(logs, unorderedEquals([...expectedLogs, contains('shouldNeverBeNull is necessary.'), contains('⚠️ Warning:')])); | ||
} else { | ||
expect(logs, unorderedEquals(expectedLogs)); | ||
} | ||
|
@@ -223,8 +267,7 @@ void main() { | |
if (runtimeSupportsPropTypeWarnings()) { | ||
group('captures errors correctly', () { | ||
test('when mounting', () { | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({'shouldAlwaysBeFalse': true}) as ReactElement), | ||
configuration: ConsoleConfig.error); | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({'shouldAlwaysBeFalse': true}) as ReactElement), configuration: ConsoleConfig.error); | ||
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. Looks like these got formatted differently by mistake 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. they bug me... so i re-formatted them in the next commit. |
||
|
||
expect( | ||
logs, | ||
|
@@ -239,15 +282,14 @@ void main() { | |
final view = rtl.render(Sample({'shouldAlwaysBeFalse': true}) as ReactElement); | ||
|
||
// Should clear the error from mounting and not create any more | ||
final logs = recordConsoleLogs(() => view.rerender(Sample({'shouldNeverBeNull': true}) as ReactElement), | ||
configuration: ConsoleConfig.error); | ||
final logs = | ||
recordConsoleLogs(() => view.rerender(Sample({'shouldNeverBeNull': true}) as ReactElement), configuration: ConsoleConfig.error); | ||
|
||
expect(logs, hasLength(0)); | ||
}); | ||
|
||
test('with nested components', () { | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample2({})) as ReactElement), | ||
configuration: ConsoleConfig.error); | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample2({})) as ReactElement), configuration: ConsoleConfig.error); | ||
|
||
expect( | ||
logs, | ||
|
@@ -258,8 +300,7 @@ void main() { | |
}); | ||
|
||
test('with nested components that are the same', () { | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample({})) as ReactElement), | ||
configuration: ConsoleConfig.error); | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample({})) as ReactElement), configuration: ConsoleConfig.error); | ||
|
||
expect( | ||
logs, | ||
|
@@ -294,8 +335,7 @@ void main() { | |
final view = rtl.render(Sample({}) as ReactElement); | ||
|
||
// Should clear the previous log and result in there being two | ||
final logs = recordConsoleLogs(() => view.rerender(Sample({'addExtraLogAndWarn': true}) as ReactElement), | ||
configuration: ConsoleConfig.log); | ||
final logs = recordConsoleLogs(() => view.rerender(Sample({'addExtraLogAndWarn': true}) as ReactElement), configuration: ConsoleConfig.log); | ||
|
||
expect( | ||
logs, | ||
|
@@ -306,8 +346,7 @@ void main() { | |
}); | ||
|
||
test('with nested components', () { | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample2({})) as ReactElement), | ||
configuration: ConsoleConfig.log); | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample2({})) as ReactElement), configuration: ConsoleConfig.log); | ||
|
||
if (runtimeSupportsPropTypeWarnings()) { | ||
expect( | ||
|
@@ -332,8 +371,7 @@ void main() { | |
}); | ||
|
||
test('with nested components that are the same', () { | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample({})) as ReactElement), | ||
configuration: ConsoleConfig.log); | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample({})) as ReactElement), configuration: ConsoleConfig.log); | ||
|
||
if (runtimeSupportsPropTypeWarnings()) { | ||
expect( | ||
|
@@ -375,8 +413,7 @@ void main() { | |
final view = rtl.render(Sample({}) as ReactElement); | ||
|
||
// Should clear the previous warnings and result in there being 3 | ||
final logs = recordConsoleLogs(() => view.rerender(Sample({'addExtraLogAndWarn': true}) as ReactElement), | ||
configuration: ConsoleConfig.warn); | ||
final logs = recordConsoleLogs(() => view.rerender(Sample({'addExtraLogAndWarn': true}) as ReactElement), configuration: ConsoleConfig.warn); | ||
|
||
expect( | ||
logs, | ||
|
@@ -388,15 +425,13 @@ void main() { | |
}); | ||
|
||
test('with nested components', () { | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample2({})) as ReactElement), | ||
configuration: ConsoleConfig.warn); | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample2({})) as ReactElement), configuration: ConsoleConfig.warn); | ||
|
||
expect(logs, hasLength(6)); | ||
}); | ||
|
||
test('with nested components that are the same', () { | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample({})) as ReactElement), | ||
configuration: ConsoleConfig.warn); | ||
final logs = recordConsoleLogs(() => rtl.render(Sample({}, Sample({})) as ReactElement), configuration: ConsoleConfig.warn); | ||
|
||
expect(logs, hasLength(6)); | ||
}); | ||
|
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.
You want me to close this PR? 😛
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.
🖕 get out of here friday greg its wednesday.
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.
This is worse than no doc comment lol