Image-sequencer: 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[bot]]

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

[gitmate[bot]]

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

[Mridul97]

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

[VibhorCodecianGupta]

@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.

[VibhorCodecianGupta]

@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.

[VibhorCodecianGupta]

@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.

[VibhorCodecianGupta]

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 :laughing: 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 [e-mail]
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
https://github.com/publiclab/image-sequencer/issues/659#issuecomment-473709569,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJ4Tckpp63cu0EtcbNRodKB_9Amr2ks5vXqAAgaJpZM4Z8fNE
.

[harshkhandeparkar]

Hmm, i'll try.

[jywarren]

Linking to #1550 and https://github.com/publiclab/image-sequencer/issues/1113 !

[jywarren]

Fixed in #1718