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.

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

string-test: fix localCompare test #4632

Merged
merged 1 commit into from
Feb 1, 2018

Conversation

obastemur
Copy link
Collaborator

spec asks for <, =, or > while our baseline was expecting a particular output.

@obastemur
Copy link
Collaborator Author

/cc @jackhorton

@rhuanjl
Copy link
Collaborator

rhuanjl commented Feb 1, 2018

I was confused that 1.9 and Master output 0, 48 and -20 for these cases whilst 1.8 still outputs 0, 1 and -1. (Considering the only relevant PR I could see was put into all 3)

@@ -105,7 +105,7 @@
<test>
<default>
<files>compare.js</files>
<baseline>compare.baseline</baseline>
<baseline/>
Copy link
Contributor

Choose a reason for hiding this comment

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

If there's no baseline now, I'd prefer to remove the <baseline/> tag completely.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't have a strong op. on that, so removed.

@obastemur
Copy link
Collaborator Author

@MSLaguana Thanks for the review!

@chakrabot chakrabot merged commit 497d3d9 into chakra-core:release/1.9 Feb 1, 2018
chakrabot pushed a commit that referenced this pull request Feb 1, 2018
Merge pull request #4632 from obastemur:fix_test_strcmp

spec asks for `<`, `=`, or `>` while our baseline was expecting a particular output.
chakrabot pushed a commit that referenced this pull request Feb 1, 2018
Merge pull request #4632 from obastemur:fix_test_strcmp

spec asks for `<`, `=`, or `>` while our baseline was expecting a particular output.
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.

4 participants