Skip to content
Permalink
Browse files
Support: Add expected support results for Safari 7 & IE11
  • Loading branch information
@mgol
mgol committed Jan 22, 2014
1 parent c11becb commit 37d036a84cfcd39e3ca7723540c6a6791d58fc69
Showing 1 changed file with 32 additions and 0 deletions.
32 test/unit/support.js
View file
@@ -83,6 +83,22 @@ testIframeWithCallback( "Check CSP (https://developer.mozilla.org/en-US/docs/Sec
"radioValue": false,
"reliableMarginRight": true
};
} else if ( /trident\/7\.0/i.test( userAgent ) ) {
expected = {
"ajax": true,
"boxSizingReliable": false,
"checkClone": true,
"checkOn": true,
"clearCloneStyle": false,
"cors": true,
"focusinBubbles": true,
"noCloneChecked": true,
"optDisabled": true,
"optSelected": false,
"pixelPosition": true,
"radioValue": false,
"reliableMarginRight": true
};
} else if ( /msie 10\.0/i.test( userAgent ) ) {
expected = {
"ajax": true,
@@ -115,6 +131,22 @@ testIframeWithCallback( "Check CSP (https://developer.mozilla.org/en-US/docs/Sec
"radioValue": false,
"reliableMarginRight": true
};
} else if ( /7\.0\.\d+ safari/i.test( userAgent ) ) {
expected = {
"ajax": true,
"boxSizingReliable": true,
"checkClone": true,
"checkOn": true,
"clearCloneStyle": true,
"cors": true,
"focusinBubbles": false,
"noCloneChecked": true,
"optDisabled": true,
"optSelected": true,
"pixelPosition": false,
"radioValue": true,
"reliableMarginRight": true
};
} else if ( /6\.0\.\d+ safari/i.test( userAgent ) ) {
expected = {
"ajax": true,

11 comments on commit 37d036a

@markelog
Copy link
Member

Choose a reason for hiding this comment

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

You changed only tests here, so i guess "Tests" component would be more appropriate.

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

I think the Test module should be reserved for changes to the actual testing infrastructure. If you were filing a bug for this, you'd file it under Support.

@markelog
Copy link
Member

Choose a reason for hiding this comment

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

That's because we don't have "tests" component at the tracker, when i saw these two last commits, one under "Tests" and one under "Support" made me think the one of them actually changed the source and changing the source is usually a big deal, so for me, it would be more useful to instantaneously know if we changed the source or not by just looking at the commit head.

@mgol
Copy link
Member Author

@mgol mgol commented on 37d036a Jan 23, 2014

Choose a reason for hiding this comment

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

Yeah, I mixed it up a little. First, there is the Support component but not the Test one which made me mark this commit as such but then I agree with @markelog having Tests at the beginning of a commit message is helpful as it shows if the change was made purely to tests - such changes can be introduce at any time, even between RC & stable as they're safe contrary to changes in src/. It adds value to be able to quickly judge if a particular set of commits is safe in this way just by looking at beginnings of commit messages; in this way @dmethvin could quickly see if we didn't break his request to not land any code changes before the release.

@timmywil @dmethvin What do you think?

@markelog
Copy link
Member

Choose a reason for hiding this comment

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

Well, i wouldn't call them "safe", since in the perfect world we should release only if we all green, if we would broke the test, it would put a hold on the release, or we could fix the test and it would actually show the error, see this for reference.

@timmywil
Copy link
Member

Choose a reason for hiding this comment

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

I'm inclined to agree with @scottgonzalez, strangely enough. "Support" adds specificity about which module is being tested, whereas "Tests" is a generic component related to test efficiency and stability. Dave can trust you not to make source changes.

@timmywil
Copy link
Member

Choose a reason for hiding this comment

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

That said, @markelog has a point. We should still be especially careful about test changes right before a release.

@timmywil
Copy link
Member

Choose a reason for hiding this comment

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

In this particular case, landing it was the right call. Here you are adding better test coverage, which is good to verify before a release.

@mgol
Copy link
Member Author

@mgol mgol commented on 37d036a Jan 23, 2014

Choose a reason for hiding this comment

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

OK, maybe you're right. As for being careful, that's exactly why I landed this commit which only adds support matrices for 2 browsers and I wouldn't land #1496 by myself without consulting even if the workaround was less crappy.

@markelog
Copy link
Member

Choose a reason for hiding this comment

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

Dave can trust you not to make source changes.

Not everyone could be aware, if, at some point, Commiter and Dave would agree to make some change before the release. I'm saying if i would see "Tests" component, i wouldn't be much in the hurry to look at it, but if i would see name of some the module, i would really want to check it out right away.

@timmywil
Copy link
Member

Choose a reason for hiding this comment

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

@markelog I understand, but the usefulness of "Support" as the affected component outweighs the occasional usefulness of skimming commit messages before a release.

Please sign in to comment.