Skip to content
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.

Sign up for GitHub

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

Jump to bottom

UIP-2414: Resolve ddc test failures #122

Merged
merged 2 commits into from
Jun 23, 2017
Merged

UIP-2414: Resolve ddc test failures #122

merged 2 commits into from
Jun 23, 2017

Conversation

clairesarsam-wf
Copy link
Contributor

@clairesarsam-wf clairesarsam-wf commented Jun 20, 2017 •
edited

Ultimate Problem

Several dozen tests failed when the test code was compiled using dartdevc.

Solution

How to +10/QA

  1. All tests should pass when run normally:
pub run test -p content-shell,chrome

Note: I had to do this on 1.23.0.
2. All tests should pass when run like this:
First add the following to pubspec.yaml:

transformers:
- test/pub_serve:
    $include: test/**_test.dart

Then, run:

$ pub serve test --web-compiler=dartdevc
# in a second window
pub run test -p chrome --pub-serve=8080 -x ddcFailure

Please review @aaronlademann-wf @greglittlefield-wf @jacehensley-wf

@jacehensley-wf jacehensley-wf mentioned this pull request Jun 22, 2017
Merged
greglittlefield-wf
greglittlefield-wf reviewed Jun 23, 2017
View reviewed changes
test/factory/common_factory_tests.dart Outdated
@@ -33,7 +35,7 @@ void commonFactoryTests(Function factory) {
test('multiple arguments', () {
var instance = factory({}, 'one', 'two');
expect(getJsChildren(instance), equals(['one', 'two']));
});
}, tags: 'ddcFailure');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should have a comment as to why it's disabled in the DDC

test/factory/common_factory_tests.dart Outdated
@@ -155,7 +157,7 @@ void _childKeyWarningTests(Function factory) {
);

expect(consoleErrorCalled, isFalse, reason: 'should not have outputted a warning');
});
}, tags: 'ddcFailure');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should have a comment as to why it's disabled in the DDC

test/factory/common_factory_tests.dart Outdated
@@ -1,3 +1,5 @@
@Tags(const ["ddcFailure"])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this needed? Wouldn't this mark the whole suite as ddcFailure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I misread the docs about this.. for some reason I thought I needed to declare the tag to be able to use it. I'll take this out.

@clairesarsam-wf
Copy link
Contributor Author

@greglittlefield-wf Feedback addressed

@jacehensley-wf
Copy link
Contributor

+1

@greglittlefield-wf
Copy link
Collaborator

  • All tests pass when run normally in content_shell and dart2js in Dart 1.23.0
  • All tests pass when run in the DDC, as per instructions
  • Smoke tested rendering in WSD examples, no regressions found

QA +10

@greglittlefield-wf greglittlefield-wf merged commit ad36450 into Workiva:master Jun 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greglittlefield-wf greglittlefield-wf greglittlefield-wf left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@clairesarsam-wf @jacehensley-wf @greglittlefield-wf