Fix build breakage introduced in npm syncing

[chadwhitacre]

Should address at least some of #4172 (comment).

[chadwhitacre]

I don't like that our tests could silently pass if one of our dependencies goes missing at Travis, but I don't know of a good way to address that easily.

[chadwhitacre]

Ready for review and merge, @nobodxbodon @kaguillera.

[nobodxbodon]

LGTM. I wonder how review process works? The committer waits for at least one contributor giving green light?

[chadwhitacre]

@nobodxbodon Yeah, if it looks good to you then you can go ahead and merge! :-)

[nobodxbodon]

Sorry about the premature merging. I just realized that I overlooked the python parts. As I'm not that familiar with Travis, @kaguillera please review again.
@whit537 how could Travis miss any of our dependencies? Does that mean our code base replies on some thirty-party library that's not recognized by Travis?

[chadwhitacre]

how could Travis miss any of our dependencies? Does that mean our code base replies on some thirty-party library that's not recognized by Travis?

We are installing the necessary dependencies at Travis. My concern was that if that dependency installation started failing for some reason, we wouldn't notice right away, because we would simply start silently skipping said tests at Travis.

[kaguillera]

@nobodxbodon as you said it looks good to me. @whit537 as far as I see it will only ignore those test are associated with ijson and marky. That seems appropriate.

[rohitpaulk]

Umm shouldn't we be ensuring that those dependencies are installed on Travis? We've got full root access, what prevents us from doing so?

[chadwhitacre]

@rohitpaulk They are installed on Travis today. My point was just that if the deps installation ever breaks (e.g., the places we're installing from change), then we wouldn't notice because the tests would just be skipped and Travis would still be green. No?

[rohitpaulk]

I just read through the entire thread - what I meant to say is "Why make this change at all?". If tests are failing for anyone, the right way to fix it is to install the dependencies (which we could include in our bootstrap scripts) - not to skip tests.

[rohitpaulk]

we wouldn't notice because the tests would just be skipped and Travis would still be green. No?

Exactly, this is the part that I don't like :)

[nobodxbodon]

If tests are failing for anyone, the right way to fix it is to install the dependencies (which we could include in our bootstrap scripts) - not to skip tests.

+1 is there a way to config Travis so missing dependency leads to failure instead of skipping? Feels like a common need to me.

[chadwhitacre]

the right way to fix it is to install the dependencies

Yeah, but yajl and npm are so heavy. 😞 Options:

• Allow for the non-yajl ijson (less performant).
• Move this back out into gdr.rocks.