refactor: convert samples test from ava to mocha

[nareshqlogic]

Fixes convert all sample tests from ava to mocha #2865(googleapis/google-cloud-node#2865) (it's a good idea to open an issue first for discussion)

• Tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

[DominicKramer]

Thank you for opening this PR. For completeness, can you please provide a short summary of why this change is needed for those that don't want to log in to view the link you referenced. Thanks.

[JustinBeckwith]

@DominicKramer this is same as the other, we're trying to consistently use mocha for all the tests.

[DominicKramer]

@JustinBeckwith ok sounds good 👍

[nareshqlogic]

Hi @JustinBeckwith ,
Sample test is getting failed because the value of constant APP_LOG_SUFFIX is undefined as there is no property APP_LOG_SUFFIX in lb.express object. So let me know can I remove two lines
const lb = require('@google-cloud/logging-bunyan');
const {APP_LOG_SUFFIX} = lb.express;
and also change the line no 48
from const log = logging.log(samples_express_${APP_LOG_SUFFIX});
to const log = logging.log(samples_express);
Kindly provide your feedback.

[JustinBeckwith]

👋 Thanks for the heads up. I think someone on @googleapis/node-team may be better suited to answer this though :)

[ofrobots]

@JustinBeckwith the export APP_LOG_SUFFIX was added in 5ab8724. This has not been released yet.

I guess what this implies is that we cannot update the samples at the same time as adding a feature to the library – because the samples can only test released versions. IMHO, it would be unnecessary hardship for features to be split so that the sample change happens separately after the feature gets released. Either we have to figure out a better way to test samples, or live with the fact that samples will be broken until a new feature gets shipped in a release.

TL;DR: this should get fixed after a new release gets shipped.

[JustinBeckwith]

@ofrobots That shouldn't be the case. The script we use to test samples does an npm link ../ in the samples directory, pulling in the sources.

[ofrobots]

@JustinBeckwith can you point to the script? If what you say is true, then that script is no longer working correctly.

[JustinBeckwith]

Sure - that's done in all of our CircleCI scripts here:
https://github.com/googleapis/nodejs-logging-bunyan/blob/master/.circleci/config.yml#L88

Is it possible it isn't happening in the kokoro script @kinwa91?

[JustinBeckwith]

Actually looks like it's happening here:
https://github.com/googleapis/nodejs-logging-bunyan/blob/master/.kokoro/samples-test.sh#L38

[ofrobots]

The error is unexplainable then. I'm looking into it further =)

[ofrobots]

So here, an issue.. I am not sure if it is a problem yet.

nodejs-logging-bunyan/.kokoro/samples-test.sh

Line 38 in 202571b

npm link ../

This code installs a link. Then we run npm run samples-tests in the parent directory.

nodejs-logging-bunyan/.kokoro/samples-test.sh

Line 42 in 202571b

npm run samples-test

Which installs the link again.

nodejs-logging-bunyan/package.json

Line 45 in 202571b

"samples-test": "cd samples/ && npm link ../ && npm test && cd ../",

[ofrobots]

The sample test seems to be passing on the latest run even though the status has been updated here.

[JustinBeckwith]

The system test failures appear to be unrelated, and I filed an issue in #204 to track that part.