CLI error did not trigger a Travis failure

[jywarren]

In #645 we noticed a bug in the CLI version broke that interface - the bug appeared in Travis but did not cause Travis to fail, unfortunately, so we didn't notice it!

The original error was fixed, but actually there seems to be a new one in the CLI, here:

https://travis-ci.org/publiclab/image-sequencer/jobs/478649385#L644-L658

We should ensure that the CLI tests actually detect and report a failure in Travis. @Mridul97 you mentioned you may be interested in working on this one? Thanks!

[gitmate]

GitMate.io thinks possibly related issues are #645 (CLI NOT WORKING), #637 (DrawRectangle info.json causes JSON parse error during Travis CI test), #145 ("reader is not defined" error on demo), #537 (Module not found error on demo), and #278 (Module not found error in demo).

[gitmate]

GitMate.io thinks possibly related issues are #645 (CLI NOT WORKING), #637 (DrawRectangle info.json causes JSON parse error during Travis CI test), #145 ("reader is not defined" error on demo), #537 (Module not found error on demo), and #278 (Module not found error in demo).

[Mridul97]

@jywarren Yes, I would like to give this one a try!

[vibhorgupta-gh]

@jywarren @Mridul97 check this out. I just made a PR at #665 and it fails the Travis CI.

https://travis-ci.org/publiclab/image-sequencer/jobs/478709461#L1710

Apparently, CI is now failing for the syntax error it wasn't beforehand

UPDATE

Pushed a commit to #665 to fix the build
Travis seems to not catch just the Syntax errors then (?)

[harshkhandeparkar]

@VibhorCodecianGupta your PR has other errors too. The problem is that line 1710 fails each time but the CI test doesn't fail.

[vibhorgupta-gh]

@harshkhandeparkar yes, I fixed them. So Travis is not catching the syntax errors then I presume?

[harshkhandeparkar]

@VibhorCodecianGupta your PR tests are failing because of many reasons but that line 1710 fails in every PR but the overall test does not fail.

[vibhorgupta-gh]

@harshkhandeparkar I fixed my PR which was failing because of a parsing error. I noticed the line 1710 failing and another similar error (which is now gone because of the fix) that of using let outside the strict scope. Which is why I was wondering whether it's just Syntax errors that Travis is not catching, but apparently not, as https://travis-ci.org/publiclab/image-sequencer/jobs/478709461#L1710 is a problem too.

[harshkhandeparkar]

@VibhorCodecianGupta what about https://travis-ci.org/publiclab/image-sequencer/jobs/478484508? The build is not failing even though error is thrown.

[vibhorgupta-gh]

That is exactly what I'm saying 😆
I thought it was for all syntax errors like it is in this one

@VibhorCodecianGupta what about https://travis-ci.org/publiclab/image-sequencer/jobs/478484508? The build is not failing even though error is thrown.

but apparently it isn't as can be seen here

The original error was fixed, but actually there seems to be a new one in the CLI, here:
https://travis-ci.org/publiclab/image-sequencer/jobs/478649385#L644-L658

So the issue is somewhere else

[harshkhandeparkar]

That is exactly what I'm saying
I thought it was for all syntax errors like it is in this one

@VibhorCodecianGupta what about https://travis-ci.org/publiclab/image-sequencer/jobs/478484508? The build is not failing even though error is thrown.

but apparently it isn't as can be seen here

The original error was fixed, but actually there seems to be a new one in the CLI, here:
https://travis-ci.org/publiclab/image-sequencer/jobs/478649385#L644-L658

So the issue is somewhere else

LOL 😆 I thought you were saying something else.

[harshkhandeparkar]

How about dropping support for node 6? It doesn't support the spread operator and throws an error but travis doesn't fail though. Maybe we have to run set -e which will terminate script with a non-0 if any commands which are run inside the script exit with non zero.

[jywarren]

+1 to both of these, i think they're fine; can you test out the -e in a PR? Thank you!

…

On Sun, Mar 17, 2019 at 4:05 PM Harsh Khandeparkar ***@***.***> wrote: How about dropping support for node 6? It doesn't support the spread operator and throws an error but travis doesn't fail though. Maybe we have to run set -e which will terminate script with a non-0 if any commands which are run inside the script exit with non zero. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#659 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ4Tckpp63cu0EtcbNRodKB_9Amr2ks5vXqAAgaJpZM4Z8fNE> .

[harshkhandeparkar]

Hmm, i'll try.

[jywarren]

Linking to #1550 and #1113 !

[jywarren]

Fixed in #1718