Skip to content

Create test_common package #1945

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

Merged
merged 6 commits into from
Feb 14, 2023
Merged

Conversation

annagrin
Copy link
Contributor

@annagrin annagrin commented Feb 8, 2023

Create test_common package:

  • Move TestSdkLayout with test-only sdk layout paths to test_common.
  • Move SdkAssetGenerator, logging to test_common.
  • Move dwds/test/fixtures/logging.dart to test_common.
  • Move sdk asset generator tests to test_common tests.
  • Update dwds tests to import test_common.
  • Make frontend_server_common use TestSdkLayout instead of hard coded layout strings.

Note: this change allows us to enable weak null safety tests for webdev (to be addressed in a separate PR).

Towards: #1878
Closes: #1892

@annagrin annagrin marked this pull request as draft February 8, 2023 19:04
@annagrin annagrin mentioned this pull request Feb 9, 2023
13 tasks
@annagrin annagrin force-pushed the annagrin/test-common branch from 036899b to e781731 Compare February 14, 2023 02:38
@annagrin annagrin marked this pull request as ready for review February 14, 2023 02:43
@annagrin annagrin requested a review from elliette February 14, 2023 02:43
@@ -9,14 +9,13 @@ import 'dart:convert';
import 'dart:io';

import 'package:dwds/expression_compiler.dart';
import 'package:dwds/sdk_configuration.dart';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be test_sdk_configuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one should not need any extra assets, so it is actually good to test it with the current SDK layout. I'll add a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait it is currently running with weak null safety! Will update (so then the statement that it does not need any extra assets will be true:)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

Copy link
Contributor

@elliette elliette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question but looks good!

@annagrin annagrin merged commit 41e92be into dart-lang:master Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move common test functionality into package:test_common
2 participants