Unverified Commit 212374f0 authored by Dan Field's avatar Dan Field Committed by GitHub

Fix tests for ANSI terminals (#23906)

parent 344ebe02
......@@ -6,6 +6,7 @@ import 'dart:convert';
import 'package:file/file.dart';
import 'package:file/memory.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:mockito/mockito.dart';
import 'package:flutter_tools/src/application_package.dart';
......@@ -18,6 +19,11 @@ import 'package:flutter_tools/src/ios/ios_workflow.dart';
import 'src/common.dart';
import 'src/context.dart';
final Generator _kNoColorTerminalPlatform = () => FakePlatform.fromPlatform(const LocalPlatform())..stdoutSupportsAnsi = false;
final Map<Type, Generator> noColorTerminalOverride = <Type, Generator>{
Platform: _kNoColorTerminalPlatform,
};
void main() {
group('ApkManifestData', () {
test('Select explicity enabled activity', () {
......@@ -38,13 +44,14 @@ void main() {
final BufferLogger logger = context[Logger];
expect(
logger.errorText, 'Error running io.flutter.examples.hello_world. Default activity not found\n');
});
}, overrides: noColorTerminalOverride);
});
group('PrebuiltIOSApp', () {
final Map<Type, Generator> overrides = <Type, Generator>{
FileSystem: () => MemoryFileSystem(),
IOSWorkflow: () => MockIosWorkFlow()
IOSWorkflow: () => MockIosWorkFlow(),
Platform: _kNoColorTerminalPlatform,
};
testUsingContext('Error on non-existing file', () {
final PrebuiltIOSApp iosApp =
......
......@@ -14,6 +14,8 @@ import '../src/common.dart';
import '../src/context.dart';
import '../src/mocks.dart';
final Generator _kNoAnsiPlatform = () => FakePlatform.fromPlatform(const LocalPlatform())..stdoutSupportsAnsi = false;
void main() {
final String red = RegExp.escape(AnsiTerminal.red);
final String bold = RegExp.escape(AnsiTerminal.bold);
......@@ -35,7 +37,7 @@ void main() {
expect(mockLogger.errorText, matches( r'^\[ (?: {0,2}\+[0-9]{1,3} ms| )\] Helpless!\n$'));
}, overrides: <Type, Generator> {
OutputPreferences: () => OutputPreferences(showColor: false),
Platform: () => FakePlatform()..stdoutSupportsAnsi = false,
Platform: _kNoAnsiPlatform,
});
testUsingContext('ANSI colored errors', () async {
......@@ -227,6 +229,7 @@ void main() {
Logger: () => StdoutLogger(),
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: 40, showColor: false),
Stdio: () => mockStdio,
Platform: _kNoAnsiPlatform,
});
testUsingContext('Error logs are wrapped and can be indented.', () async {
......@@ -245,6 +248,7 @@ void main() {
Logger: () => StdoutLogger(),
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: 40, showColor: false),
Stdio: () => mockStdio,
Platform: _kNoAnsiPlatform,
});
testUsingContext('Error logs are wrapped and can have hanging indent.', () async {
......@@ -263,6 +267,7 @@ void main() {
Logger: () => StdoutLogger(),
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: 40, showColor: false),
Stdio: () => mockStdio,
Platform: _kNoAnsiPlatform,
});
testUsingContext('Error logs are wrapped, indented, and can have hanging indent.', () async {
......@@ -281,6 +286,7 @@ void main() {
Logger: () => StdoutLogger(),
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: 40, showColor: false),
Stdio: () => mockStdio,
Platform: _kNoAnsiPlatform,
});
testUsingContext('Stdout logs are wrapped', () async {
......@@ -296,6 +302,7 @@ void main() {
Logger: () => StdoutLogger(),
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: 40, showColor: false),
Stdio: () => mockStdio,
Platform: _kNoAnsiPlatform,
});
testUsingContext('Stdout logs are wrapped and can be indented.', () async {
......@@ -314,6 +321,7 @@ void main() {
Logger: () => StdoutLogger(),
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: 40, showColor: false),
Stdio: () => mockStdio,
Platform: _kNoAnsiPlatform,
});
testUsingContext('Stdout logs are wrapped and can have hanging indent.', () async {
......@@ -332,6 +340,7 @@ void main() {
Logger: () => StdoutLogger(),
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: 40, showColor: false),
Stdio: () => mockStdio,
Platform: _kNoAnsiPlatform,
});
testUsingContext('Stdout logs are wrapped, indented, and can have hanging indent.', () async {
......@@ -350,6 +359,7 @@ void main() {
Logger: () => StdoutLogger(),
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: 40, showColor: false),
Stdio: () => mockStdio,
Platform: _kNoAnsiPlatform,
});
testUsingContext('Error logs are red', () async {
......@@ -399,6 +409,7 @@ void main() {
Logger: () => StdoutLogger(),
OutputPreferences: () => OutputPreferences(showColor: false),
Stdio: () => mockStdio,
Platform: _kNoAnsiPlatform,
});
testUsingContext('Stdout startProgress handle null inputs on regular terminal', () async {
......@@ -416,6 +427,7 @@ void main() {
Logger: () => StdoutLogger(),
OutputPreferences: () => OutputPreferences(showColor: false),
Stdio: () => mockStdio,
Platform: _kNoAnsiPlatform,
});
testUsingContext('SummaryStatus works when cancelled', () async {
......@@ -438,7 +450,7 @@ void main() {
// Verify that stopping or canceling multiple times throws.
expect(() { summaryStatus.cancel(); }, throwsA(isInstanceOf<AssertionError>()));
expect(() { summaryStatus.stop(); }, throwsA(isInstanceOf<AssertionError>()));
}, overrides: <Type, Generator>{Stdio: () => mockStdio});
}, overrides: <Type, Generator>{Stdio: () => mockStdio, Platform: _kNoAnsiPlatform});
testUsingContext('SummaryStatus works when stopped', () async {
summaryStatus.start();
......@@ -461,7 +473,7 @@ void main() {
// Verify that stopping or canceling multiple times throws.
expect(() { summaryStatus.stop(); }, throwsA(isInstanceOf<AssertionError>()));
expect(() { summaryStatus.cancel(); }, throwsA(isInstanceOf<AssertionError>()));
}, overrides: <Type, Generator>{Stdio: () => mockStdio});
}, overrides: <Type, Generator>{Stdio: () => mockStdio, Platform: _kNoAnsiPlatform});
testUsingContext('sequential startProgress calls with StdoutLogger', () async {
context[Logger].startProgress('AAA')..stop();
......@@ -473,6 +485,7 @@ void main() {
Logger: () => StdoutLogger(),
OutputPreferences: () => OutputPreferences(showColor: false),
Stdio: () => mockStdio,
Platform: _kNoAnsiPlatform,
});
testUsingContext('sequential startProgress calls with VerboseLogger and StdoutLogger', () async {
......@@ -488,6 +501,7 @@ void main() {
}, overrides: <Type, Generator>{
Logger: () => VerboseLogger(StdoutLogger()),
Stdio: () => mockStdio,
Platform: _kNoAnsiPlatform,
});
testUsingContext('sequential startProgress calls with BufferLogger', () async {
......@@ -497,6 +511,7 @@ void main() {
expect(logger.statusText, 'AAA\nBBB\n');
}, overrides: <Type, Generator>{
Logger: () => BufferLogger(),
Platform: _kNoAnsiPlatform,
});
});
}
......@@ -4,6 +4,7 @@
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/process.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:mockito/mockito.dart';
......@@ -86,6 +87,7 @@ void main() {
Logger: () => mockLogger,
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: 40),
Platform: () => FakePlatform.fromPlatform(const LocalPlatform())..stdoutSupportsAnsi = false
});
});
}
......
......@@ -18,6 +18,11 @@ import '../src/context.dart';
/// Test case timeout for tests involving project analysis.
const Timeout allowForSlowAnalyzeTests = Timeout.factor(5.0);
final Generator _kNoColorTerminalPlatform = () => FakePlatform.fromPlatform(const LocalPlatform())..stdoutSupportsAnsi = false;
final Map<Type, Generator> noColorTerminalOverride = <Type, Generator>{
Platform: _kNoColorTerminalPlatform,
};
void main() {
final String analyzerSeparator = platform.isWindows ? '-' : '•';
......@@ -99,7 +104,7 @@ void main() {
exitMessageContains: '2 issues found.',
toolExit: true,
);
}, timeout: allowForSlowAnalyzeTests);
}, timeout: allowForSlowAnalyzeTests, overrides: noColorTerminalOverride);
// Analyze in the current directory - no arguments
testUsingContext('working directory with local options', () async {
......@@ -126,7 +131,7 @@ void main() {
exitMessageContains: '3 issues found.',
toolExit: true,
);
}, timeout: allowForSlowAnalyzeTests);
}, timeout: allowForSlowAnalyzeTests, overrides: noColorTerminalOverride);
testUsingContext('no duplicate issues', () async {
final Directory tempDir = fs.systemTempDirectory.createTempSync('flutter_analyze_once_test_2.').absolute;
......@@ -160,7 +165,7 @@ void bar() {
} finally {
tryToDelete(tempDir);
}
});
}, overrides: noColorTerminalOverride);
testUsingContext('returns no issues when source is error-free', () async {
const String contents = '''
......@@ -177,7 +182,7 @@ StringBuffer bar = StringBuffer('baz');
} finally {
tryToDelete(tempDir);
}
});
}, overrides: noColorTerminalOverride);
testUsingContext('returns no issues for todo comments', () async {
const String contents = '''
......@@ -195,7 +200,7 @@ StringBuffer bar = StringBuffer('baz');
} finally {
tryToDelete(tempDir);
}
});
}, overrides: noColorTerminalOverride);
});
}
......
......@@ -9,6 +9,7 @@ import 'package:args/command_runner.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/net.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/cache.dart';
import 'package:flutter_tools/src/commands/create.dart';
import 'package:flutter_tools/src/dart/sdk.dart';
......@@ -22,6 +23,10 @@ import '../src/context.dart';
const String frameworkRevision = '12345678';
const String frameworkChannel = 'omega';
final Generator _kNoColorTerminalPlatform = () => FakePlatform.fromPlatform(const LocalPlatform())..stdoutSupportsAnsi = false;
final Map<Type, Generator> noColorTerminalOverride = <Type, Generator> {
Platform: _kNoColorTerminalPlatform,
};
void main() {
Directory tempDir;
......@@ -109,7 +114,7 @@ void main() {
'.ios/',
]),
throwsToolExit(message: 'Sorry, unable to detect the type of project to recreate'));
}, timeout: allowForRemotePubInvocation);
}, timeout: allowForRemotePubInvocation, overrides: noColorTerminalOverride);
testUsingContext('Will create an app project if non-empty non-project directory exists without .metadata', () async {
await projectDir.absolute.childDirectory('blag').create(recursive: true);
......@@ -438,6 +443,7 @@ void main() {
expect(sdkMetaContents, contains('/bin/cache/dart-sdk/lib/core"'));
}, overrides: <Type, Generator>{
FlutterVersion: () => mockFlutterVersion,
Platform: _kNoColorTerminalPlatform,
}, timeout: allowForCreateFlutterProject);
testUsingContext('has correct content and formatting with app template', () async {
......@@ -508,6 +514,7 @@ void main() {
expect(sdkMetaContents, contains('/bin/cache/dart-sdk/lib/core"'));
}, overrides: <Type, Generator>{
FlutterVersion: () => mockFlutterVersion,
Platform: _kNoColorTerminalPlatform,
}, timeout: allowForCreateFlutterProject);
testUsingContext('can re-gen default template over existing project', () async {
......
......@@ -5,6 +5,7 @@
import 'dart:async';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/doctor.dart';
import 'package:flutter_tools/src/vscode/vscode.dart';
......@@ -13,6 +14,11 @@ import 'package:flutter_tools/src/vscode/vscode_validator.dart';
import '../src/common.dart';
import '../src/context.dart';
final Generator _kNoColorOutputPlatform = () => FakePlatform.fromPlatform(const LocalPlatform())..stdoutSupportsAnsi = false;
final Map<Type, Generator> noColorTerminalOverride = <Type, Generator>{
Platform: _kNoColorOutputPlatform
};
void main() {
group('doctor', () {
testUsingContext('intellij validator', () async {
......@@ -34,7 +40,7 @@ void main() {
.firstWhere((ValidationMessage m) => m.message.startsWith('Flutter '));
expect(message.message, contains('Flutter plugin version 0.1.3'));
expect(message.message, contains('recommended minimum version'));
});
}, overrides: noColorTerminalOverride);
testUsingContext('vs code validator when both installed', () async {
final ValidationResult result = await VsCodeValidatorTestTargets.installedWithExtension.validate();
......@@ -49,7 +55,7 @@ void main() {
message = result.messages
.firstWhere((ValidationMessage m) => m.message.startsWith('Flutter '));
expect(message.message, 'Flutter extension version 4.5.6');
});
}, overrides: noColorTerminalOverride);
testUsingContext('vs code validator when 64bit installed', () async {
expect(VsCodeValidatorTestTargets.installedWithExtension64bit.title, 'VS Code, 64-bit edition');
......@@ -65,7 +71,7 @@ void main() {
message = result.messages
.firstWhere((ValidationMessage m) => m.message.startsWith('Flutter '));
expect(message.message, 'Flutter extension version 4.5.6');
});
}, overrides: noColorTerminalOverride);
testUsingContext('vs code validator when extension missing', () async {
final ValidationResult result = await VsCodeValidatorTestTargets.installedWithoutExtension.validate();
......@@ -80,7 +86,7 @@ void main() {
message = result.messages
.firstWhere((ValidationMessage m) => m.message.startsWith('Flutter '));
expect(message.message, startsWith('Flutter extension not installed'));
});
}, overrides: noColorTerminalOverride);
});
group('doctor with overriden validators', () {
......@@ -95,7 +101,8 @@ void main() {
'• No issues found!\n'
));
}, overrides: <Type, Generator>{
DoctorValidatorsProvider: () => FakeDoctorValidatorsProvider()
DoctorValidatorsProvider: () => FakeDoctorValidatorsProvider(),
Platform: _kNoColorOutputPlatform,
});
});
......@@ -112,7 +119,7 @@ void main() {
'\n'
'• No issues found!\n'
));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate non-verbose output format when only one category fails', () async {
expect(await FakeSinglePassingDoctor().diagnose(verbose: false), isTrue);
......@@ -123,7 +130,7 @@ void main() {
'\n'
'! Doctor found issues in 1 category.\n'
));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate non-verbose output format for a passing run', () async {
expect(await FakePassingDoctor().diagnose(verbose: false), isTrue);
......@@ -139,7 +146,7 @@ void main() {
'\n'
'! Doctor found issues in 2 categories.\n'
));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate non-verbose output format', () async {
expect(await FakeDoctor().diagnose(verbose: false), isFalse);
......@@ -160,7 +167,7 @@ void main() {
'\n'
'! Doctor found issues in 4 categories.\n'
));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate verbose output format', () async {
expect(await FakeDoctor().diagnose(verbose: true), isFalse);
......@@ -190,7 +197,7 @@ void main() {
'\n'
'! Doctor found issues in 4 categories.\n'
));
});
}, overrides: noColorTerminalOverride);
});
testUsingContext('validate non-verbose output wrapping', () async {
......@@ -224,6 +231,7 @@ void main() {
));
}, overrides: <Type, Generator>{
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: 30),
Platform: _kNoColorOutputPlatform,
});
testUsingContext('validate verbose output wrapping', () async {
......@@ -268,6 +276,7 @@ void main() {
));
}, overrides: <Type, Generator>{
OutputPreferences: () => OutputPreferences(wrapText: true, wrapColumn: 30),
Platform: _kNoColorOutputPlatform,
});
......@@ -285,7 +294,7 @@ void main() {
'\n'
'! Doctor found issues in 1 category.\n'
));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate merging assigns statusInfo and title', () async {
// There are two subvalidators. Only the second contains statusInfo.
......@@ -297,7 +306,7 @@ void main() {
'\n'
'• No issues found!\n'
));
});
}, overrides: noColorTerminalOverride);
});
......@@ -309,47 +318,47 @@ void main() {
testUsingContext('validate installed + installed = installed', () async {
expect(await FakeSmallGroupDoctor(installed, installed).diagnose(), isTrue);
expect(testLogger.statusText, startsWith('[✓]'));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate installed + partial = partial', () async {
expect(await FakeSmallGroupDoctor(installed, partial).diagnose(), isTrue);
expect(testLogger.statusText, startsWith('[!]'));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate installed + missing = partial', () async {
expect(await FakeSmallGroupDoctor(installed, missing).diagnose(), isTrue);
expect(testLogger.statusText, startsWith('[!]'));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate partial + installed = partial', () async {
expect(await FakeSmallGroupDoctor(partial, installed).diagnose(), isTrue);
expect(testLogger.statusText, startsWith('[!]'));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate partial + partial = partial', () async {
expect(await FakeSmallGroupDoctor(partial, partial).diagnose(), isTrue);
expect(testLogger.statusText, startsWith('[!]'));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate partial + missing = partial', () async {
expect(await FakeSmallGroupDoctor(partial, missing).diagnose(), isTrue);
expect(testLogger.statusText, startsWith('[!]'));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate missing + installed = partial', () async {
expect(await FakeSmallGroupDoctor(missing, installed).diagnose(), isTrue);
expect(testLogger.statusText, startsWith('[!]'));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate missing + partial = partial', () async {
expect(await FakeSmallGroupDoctor(missing, partial).diagnose(), isTrue);
expect(testLogger.statusText, startsWith('[!]'));
});
}, overrides: noColorTerminalOverride);
testUsingContext('validate missing + missing = missing', () async {
expect(await FakeSmallGroupDoctor(missing, missing).diagnose(), isFalse);
expect(testLogger.statusText, startsWith('[✗]'));
});
}, overrides: noColorTerminalOverride);
});
}
......
......@@ -8,6 +8,7 @@ import 'dart:convert';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/context.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/compile.dart';
import 'package:mockito/mockito.dart';
......@@ -16,6 +17,8 @@ import 'package:process/process.dart';
import 'src/common.dart';
import 'src/context.dart';
final Generator _kNoColorTerminalPlatform = () => FakePlatform.fromPlatform(const LocalPlatform())..stdoutSupportsAnsi = false;
void main() {
group('batch compile', () {
ProcessManager mockProcessManager;
......@@ -58,6 +61,7 @@ void main() {
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Logger: () => BufferLogger(),
Platform: _kNoColorTerminalPlatform,
});
testUsingContext('single dart failed compilation', () async {
......@@ -81,6 +85,7 @@ void main() {
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Logger: () => BufferLogger(),
Platform: _kNoColorTerminalPlatform,
});
testUsingContext('single dart abnormal compiler termination', () async {
......@@ -107,6 +112,7 @@ void main() {
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Logger: () => BufferLogger(),
Platform: _kNoColorTerminalPlatform,
});
});
......@@ -165,6 +171,7 @@ void main() {
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Logger: () => BufferLogger(),
Platform: _kNoColorTerminalPlatform,
});
testUsingContext('single dart compile abnormally terminates', () async {
......@@ -182,6 +189,7 @@ void main() {
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Logger: () => BufferLogger(),
Platform: _kNoColorTerminalPlatform,
});
testUsingContext('compile and recompile', () async {
......@@ -211,6 +219,7 @@ void main() {
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Logger: () => BufferLogger(),
Platform: _kNoColorTerminalPlatform,
});
testUsingContext('compile and recompile twice', () async {
......@@ -241,6 +250,7 @@ void main() {
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Logger: () => BufferLogger(),
Platform: _kNoColorTerminalPlatform,
});
});
......@@ -333,6 +343,7 @@ void main() {
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Logger: () => BufferLogger(),
Platform: _kNoColorTerminalPlatform,
});
testUsingContext('compile expressions without awaiting', () async {
......@@ -398,6 +409,7 @@ void main() {
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Logger: () => BufferLogger(),
Platform: _kNoColorTerminalPlatform,
});
});
}
......
......@@ -17,6 +17,11 @@ import 'package:process/process.dart';
import '../src/common.dart';
import '../src/context.dart';
final Generator _kNoColorTerminalPlatform = () => FakePlatform.fromPlatform(const LocalPlatform())..stdoutSupportsAnsi = false;
final Map<Type, Generator> noColorTerminalOverride = <Type, Generator>{
Platform: _kNoColorTerminalPlatform,
};
class MockProcessManager extends Mock implements ProcessManager {}
class MockFile extends Mock implements File {}
class MockXcodeProjectInterpreter extends Mock implements XcodeProjectInterpreter {}
......@@ -339,7 +344,7 @@ Error launching application on iPhone.''',
testLogger.errorText,
contains('No Provisioning Profile was found for your project\'s Bundle Identifier or your \ndevice.'),
);
});
}, overrides: noColorTerminalOverride);
testUsingContext('No development team shows message', () async {
final XcodeBuildResult buildResult = XcodeBuildResult(
......@@ -420,6 +425,6 @@ Could not build the precompiled application for the device.''',
testLogger.errorText,
contains('Building a deployable iOS app requires a selected Development Team with a \nProvisioning Profile.'),
);
});
}, overrides: noColorTerminalOverride);
});
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment