Update tests that are incompatible with 3.0.0
#1817
Conversation
There was a problem hiding this comment.
Thanks Elliott, this is fantastic!
I left comments mainly to suggest running tests in all modes and skipping the ones that currently fail - since the intention is to run in both modes for now (at least for the tests that were doing it before), it would be easier to fix and follow.
| var sdkSourceMap = | ||
| soundNullSafety ? dartSdkSourcemapSound : dartSdkSourcemap; | ||
| final sdkVersion = Version.parse(Platform.version.split(' ')[0]); | ||
| var sdk = |
There was a problem hiding this comment.
Suggestion: The sdk loaded should only depend on the mull safety mode, not the dart version, to avoid mismatches between the test mode and the sdk loaded. If it fails in some tests, we should just disable those test with a reference to an issue.
There was a problem hiding this comment.
Removed changes to this file, will skip any tests that fail
| @@ -44,7 +49,7 @@ void main() { | |||
| // Enable verbose logging for debugging. | |||
| final debug = false; | |||
|
|
|||
| for (var nullSafety in NullSafety.values) { | |||
| for (var nullSafety in supportedNullSafetyModes) { | |||
There was a problem hiding this comment.
We should be able to run with weak null safety and 3.0.0 for a while (maybe a special flag is needed for the frontend?) so I suggest to run for all modes and skip the ones that are not working currently with a reference to an issue.
There was a problem hiding this comment.
Switched to using NullSafety.values (here and elsewhere) and skipping with a TODO
| @@ -838,7 +809,7 @@ void main() { | |||
| expect(obj.kind, InstanceKind.kNull); | |||
| expect(obj.classRef!.name, 'Null'); | |||
| expect(obj.valueAsString, 'null'); | |||
| }); | |||
| }, skip: true); | |||
| setUp(() async { | ||
| // TODO(https://github.com/dart-lang/webdev/issues/1591): Frontend server | ||
| // compilation is currently incompatible with sound null safety. | ||
| if (supportedNullSafetyModes.contains(NullSafety.weak)) { |
There was a problem hiding this comment.
This should be able to run in dart 3 with weak null safety - can we try passing the null safety argument to the ResidentCompiler and --sound-null-safety and --no-sound-null-safety arguments to the frontend server snapshot, depending on the null safety mode? If it still does not work, I think it is better to run it here for any SDK and then just skip the tests with an issue reference for the dart 3.0.0 sdk.
There was a problem hiding this comment.
Tried getting it to work, and looks like it will require more changes. Added as one of the TODO steps in #1818 so that it can be tackled in a separate PR
|
|
||
| import '../fixtures/context.dart'; | ||
|
|
||
| bool get versionSupportsWeakNullSafety => |
There was a problem hiding this comment.
Dart 3 should support weak mode for some time (maybe under a special flag?) so maybe we can change the name to something along the lines of dart2 or dartMajorVersion2? Or even just return the major version number so we can check it in tests?
There was a problem hiding this comment.
I've removed versionSupportsWeakNullSafet
Work towards #1818
This addresses the first task, and is primarily to unblock the broken CI tests at head. It fixes what is fairly non-trivial to fix, and disables tests that will take more work.