Re: [PATCH v2 4/4] t4018: add tests for javascript export type function declarations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Many of the cases are not valid tests. In particular the word "ChangeMe"
must occur *only once* and *after* the line with "RIGHT" and there must
be at least one unchanged line before it. (I wonder how these tests
could have passed. Do we have a flaw in the test driver?)

> diff --git a/t/t4018/javascript-dotexpors-async-anonymous-function b/t/t4018/javascript-dotexpors-async-anonymous-function
> new file mode 100644
> index 0000000000..9f970a2343
> --- /dev/null
> +++ b/t/t4018/javascript-dotexpors-async-anonymous-function
> @@ -0,0 +1,3 @@
> +exports.RIGHT = async function(a, b) {
> +    return a + b; // ChangeMe
> +};

Here.

> diff --git a/t/t4018/javascript-dotexports-anonymous-function b/t/t4018/javascript-dotexports-anonymous-function
> new file mode 100644
> index 0000000000..2fa9775c95
> --- /dev/null
> +++ b/t/t4018/javascript-dotexports-anonymous-function
> @@ -0,0 +1,3 @@
> +exports.RIGHT = function(a, b) {
> +    return a + b; //ChangeMe
> +};

Here.

> diff --git a/t/t4018/javascript-dotexports-arrow-function-3 b/t/t4018/javascript-dotexports-arrow-function-3
> new file mode 100644
> index 0000000000..cc3f1ec017
> --- /dev/null
> +++ b/t/t4018/javascript-dotexports-arrow-function-3
> @@ -0,0 +1 @@
> +exports.RIGHT = a => a+1; //ChangeMe

Here.

And many more. You see the pattern.

> +++ b/t/t4018/javascript-module-dotexports-generator-function
> @@ -0,0 +1,5 @@
> +module.exports.RIGHT = function* ChangeMe() {
> +
> +    yield 1;
> +    yield 2;
> +}
> \ No newline at end of file

An incomplete last line, again. Please look at the patch text that you
are going to submit. Don't depend on reviewers' to notice such obvious
glitches.

I haven't found enough time for a complete review, yet. Please submit a
patch series that does not depend on an earlier round.

Splitting the test cases in separate patches is a good idea. I think you
have chosen a split that excercises different hunk header patterns. But
in this case, it would also be feasible and helpful to exclude the
pattern from the earlier patch and add the pattern and the corresponding
test cases in the same commit.

-- Hannes





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux