Add relation and equality operators by matedabis · Pull Request #17 · szeged/js-test-generators

matedabis · 2019-02-08T08:32:23Z

This generator tests ES v5.1 11.8. - 11.9. (except 11.8.7. and 11.8.6.), relation and equality operators (> >= <= < == === != !==).
The new validate function:

function validate_boolean(test_value, expected_result, false_result) {
    assert(test_value === expected_result);
    assert(test_value.toString() === expected_result.toString());
    assert(test_value !== false_result);
    assert(test_value.toString() !== false_result.toString());
}

A test case looks like this:

t1 = false;
t2 = null;
t3 = null;
t4 = false;
t5 = -2933530887918864;
t6 = false;
t7 = NaN;
t8 = '5871211041646662';
t9 = null;
t10 = NaN;
t11 = 5820634256891770;
f = true;
e = false;
validate_boolean(((((((((((t1 != t2) > t3) <= t4) <= t5) !== t6) !== t7) !== t8) >= t9) === t10) > t11)
, e, f);

rerobika · 2019-02-08T08:46:20Z

-                 declaration_string += "%s = '%s';\n" % (str(key), str(value))
-             else:
-                 declaration_string += "%s = %d;\n" % (str(key), value)
+             declaration_string += "%s = %s;\n" % (str(key), str(value))


This utility function is changes over and over the PRs. This should be finalized somehow.

rerobika · 2019-02-08T08:47:40Z

+
+    class _InnerValues():
+        generated_filename = '''relation-equality-ops-{NUMBER}.js''' # The file name prefix of the generated
+        relational_and_equality_operators = ["equ", "nequ", "sequ", "snequ", "less", "moreorequ", "more", "lessorequ"]


I'd suggest the: eq, neq, less, less_eq, greater, greater_eq namings.

rerobika · 2019-02-08T08:49:10Z

+            elif (decider == 3):
+                self.operands[i] = "false"
+            elif (decider == 4):
+                self.operands[i] = "NaN"


How about this?

if decider <= 4: self.operands[i] = special_values[i]

rerobika · 2019-02-08T08:50:32Z

+            converted = "true"
+        else:
+            converted = "false"
+        return converted


I'd suggest: return "true" if value else "false";

rerobika · 2019-02-08T08:51:22Z

+            converted = "false"
+        return converted
+
+    # Add comas to the value if it is meant to be a string in the test case


Typo: comas

rerobika · 2019-02-08T08:53:05Z

+        (left_value == "undefined" and right_value == "null") or
+        (left_value == "null" and right_value == "undefined") or
+        (left_value == "true" and right_value == "true") or
+        (left_value == "false" and right_value == "false")):


Create a mapping or something for these special cases. This look like a but ugly.

matedabis · 2019-02-08T09:59:54Z

Updated the patch.

loki04 · 2019-02-11T07:16:03Z

+
+    class _InnerValues():
+        generated_filename = '''relation-equality-ops-{NUMBER}.js''' # The file name prefix of the generated
+        relational_and_equality_operators = ["eq", "neq", "seq", "sneq", "less", "less_eq", "greater", "greater_eq"]


The assembly-like form would be much better. Please use the followings or the lowercase version of them instead!
EQ = equal
NE = not equal
GT = greater
LT = lesser
GE = greater and equal
LE = lesser and equal
SE (or STRICT_EQ) = strict equal
SN (or STRICT_NQ) = not strict equal

loki04 · 2019-02-11T07:27:55Z

+              "seq": self.seq,
+              "neq": self.neq,
+              "sneq": self.sneq,
+        } # Functions for the operators


How about to generate getattr(self, relational_and_equality_operators[i]) for each elements of relational_and_equality_operators ?

loki04 · 2019-02-11T07:29:11Z

+        } # Functions for the operators
+
+    # Generating the expression with operands and operators
+    def expression_maker(self):


expression_maker? The expression_generator sounds better.

loki04 · 2019-02-11T08:53:28Z

+
+        # If it does not match the cases above and values are not undefined or null
+        elif (left_value != "undefined" and left_value != "null" and right_value != "null"
+        and right_value != "undefined"):


elif left_value not in ["undefined", "null"] and right_value not in ["undefined", "null"]: ?

loki04 · 2019-02-11T09:04:47Z

+            result =  left_converted < right_converted
+        else:
+            result = int(left_converted) < int(right_converted)
+            result = self.convert_true_false_to_lowercase_string(result)


Please combine these expressions!

loki04 · 2019-02-11T09:06:22Z

+    # Greater than operator
+    def greater(self, left_value, right_value):
+        if (left_value == "NaN" or right_value == "NaN" or
+        left_value == "undefined" or right_value == "undefined"):


?
if left_value in ["NaN", "undefined"] or right_value in ["NaN", "undefined"]:

loki04 · 2019-02-11T09:06:53Z

+    # Less than or equal operator
+    def less_eq(self, left_value, right_value):
+        if (left_value == "NaN" or right_value == "NaN" or
+        left_value == "undefined" or right_value == "undefined"):


?
if left_value in ["NaN", "undefined"] or right_value in ["NaN", "undefined"]:

loki04 · 2019-02-11T09:07:00Z

+    # Greater than or equal operator
+    def greater_eq(self, left_value, right_value):
+        if (left_value == "NaN" or right_value == "NaN" or
+        left_value == "undefined" or right_value == "undefined"):


?
if left_value in ["NaN", "undefined"] or right_value in ["NaN", "undefined"]:

loki04 · 2019-02-11T09:10:33Z

+        write_file(gen_settings, self.file_output)
+        self.debug(Messages.done, self.options)
+
+def main():


Why do you create separate main function? Do you want to call this why you are importing this class?
If you do not want that the main will be callable, please move its body under if __name__ == '__main__': block!

matedabis · 2019-02-12T15:00:17Z

Updated the patch according to the review, and also found and fixed some bugs. I corrected almost everything mentioned, I have to reply to some questions.

expression_maker? The expression_generator sounds better.

I already have a function with that name so its not possible.

?
left_converted = {"true": 1, "false": 0}.get(left_value, int(left_value))

I had to check for true or false before, because it would try to convert the default value to int, and it throws error, this way it is still shorter and simpler than before.

if left_value in ["true", "false"]:
    left_converted = {"true": 1, "false": 0}.get(left_value)
else:
    left_converted = int(left_value)

Why do you create separate main function? Do you want to call this why you are importing this class?
If you do not want that the main will be callable, please move its body under if __name__ == '__main__': block!

I did not want it to make it callable, but this review gave me and idea so I left it as it was, but if you do not agree I will change it. When we reach the end of this project, and we will have more generators, we could create a script which calls all the generators and they each generate the same number of test cases which number is given by argparser, and we could add a command line option to run the generated tests with the test runner script after the generators finished generating, and maybe an another one, which deletes the .js files which did not fail (This would not be useful in all the cases, but as an option). And if this project seems to be useful, we could insert it to jerryscript's run-tests.py with a command line option. What do you think?

loki04 · 2019-02-13T13:59:05Z

Updated the patch according to the review, and also found and fixed some bugs. I corrected almost everything mentioned, I have to reply to some questions.

expression_maker? The expression_generator sounds better.

I already have a function with that name so its not possible.

It sounds weird. Please, try to find a different function name.

?
left_converted = {"true": 1, "false": 0}.get(left_value, int(left_value))

I had to check for true or false before, because it would try to convert the default value to int, and it throws error, this way it is still shorter and simpler than before.

Well, probably I wrote the int() at wrong position. Consider this instead:

for v in [0, 1, 42, -33, "123", "true", "false"]:
  out = int({"true": 1, "false": 0}.get(v, v))
  print(out, type(out))

0 <class 'int'>
1 <class 'int'>
42 <class 'int'>
-33 <class 'int'>
123 <class 'int'>
1 <class 'int'>
0 <class 'int'>

Why do you create separate main function? Do you want to call this why you are importing this class?
If you do not want that the main will be callable, please move its body under if __name__ == '__main__': block!

I did not want it to make it callable, but this review gave me and idea so I left it as it was, but if you do not agree I will change it. When we reach the end of this project, and we will have more generators, we could create a script which calls all the generators and they each generate the same number of test cases which number is given by argparser, and we could add a command line option to run the generated tests with the test runner script after the generators finished generating, and maybe an another one, which deletes the .js files which did not fail (This would not be useful in all the cases, but as an option). And if this project seems to be useful, we could insert it to jerryscript's run-tests.py with a command line option. What do you think?

This can be done and could be a good option, but I still can't see the need of a separate main. In your case we can import classes and use them as you described above.

rerobika · 2019-02-19T06:59:51Z

+    def eq_helper_special_cases(self, values):
+        special_cases = [["undefined", "undefined"], ["null", "null"], ["undefined", "null"],
+        ["null", "undefined"], ["true", "true"], ["false", "false"]]
+        return True if values in special_cases else False


return values in special_cases is enough

rerobika requested changes Feb 8, 2019

View reviewed changes

matedabis force-pushed the add_binary_features branch 2 times, most recently from 6683c57 to 5ca07d7 Compare February 8, 2019 09:57

loki04 requested changes Feb 11, 2019

View reviewed changes

matedabis force-pushed the add_binary_features branch from 5ca07d7 to ec29d28 Compare February 12, 2019 14:25

matedabis force-pushed the add_binary_features branch from ec29d28 to 7085814 Compare February 14, 2019 11:30

rerobika reviewed Feb 19, 2019

View reviewed changes

Add relation and equality operators

513310b

matedabis force-pushed the add_binary_features branch from 7085814 to 513310b Compare February 19, 2019 07:45

Conversation

matedabis commented Feb 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matedabis commented Feb 8, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matedabis commented Feb 12, 2019

Uh oh!

loki04 commented Feb 13, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

matedabis commented Feb 8, 2019 •

edited

Loading