fix(search): change longname to be an optional property

[pudgereyem]

This PR fixes an issue where Yahoo doesn't always return SearchQuoteYahooEquity.longname.

Issue

See #13.

Solution

Change so that SearchQuoteYahooEquity.longname is an optional property which is not always returned by Yahoo. I've also added some more tests to the search module.

Changes

• change longname to be an optional property
• add more tests to search.spec.ts
• schema.json changed after running yarn generateSchema

How to test

$ jest --verbose --silent --t search
 PASS  src/modules/search.spec.ts
  search
    ✓ passed validation for search: AAPL (75 ms)
    ✓ passed validation for search: OCDO.L (2 ms)
    ✓ passed validation for search: BABA (1 ms)
    ✓ passed validation for search: Evolution Gaming Group (1 ms)
    ✓ passed validation for search: Bayerische Motoren Werke AG (3 ms)
    ✓ throws InvalidOptions on invalid options (12 ms)

This is only possible if you comment out the custom reporters defined, see more details about this in the comment below.

Comments (for the reviewer)

Validate that the changes to schema.json are ok

After running yarn generateSchema I saw that schema.json was changed in more places than I expected. I'd appreciate a second opinion that this is all good.

Custom jest reporters prevent --verbose to output individual tests

I like to run jest --verbose so that I can see the results of the individual tests when working. However, the custom reporters that was added, see code here prevents that behaviour. In order to see the individual tests (as shown in the above section) I had to comment out those. See below.

// jest.config.js

module.exports = {
  preset: 'ts-jest',
  setupFilesAfterEnv: [
    "<rootDir>/tests/setupTests.js"
  ],
  testEnvironment: 'node',
  testPathIgnorePatterns: [
    "/node_modules/",
    "/api/"
  ],
  // reporters: [
  //   '<rootDir>/tests/reporter.js',
  //   '<rootDir>/tests/summary-reporter.js',
  // ],
};

What are your thoughts on this? I personally appreciate seeing what different tests are running.

[gadicc]

Thanks, @pudgereyem, this is fantastic. I must commend you once again on the detailed report and familiarising yourself so well with the project's internals and practices. Looking forward to many more future collaborations :)

Merging now, my response to follow.

[gadicc]

Validate that the changes to schema.json are ok

After running yarn generateSchema I saw that schema.json was changed in more places than I expected. I'd appreciate a second opinion that this is all good.

Haha so glad I added that section about generateSchema in the contributing doc before this came up! Thanks for reading it all :)

And for noticing this. It's my fault. Seems I updated the interfaces earlier in the day and forgot to regenerate the schema. That's usually something that's obvious in tests but in this case I changed something that didn't have any effect. I must think if there's another hook to force this test however it does run automatically before publishing.

[pudgereyem]

@gadicc, ah thanks man, appreciate it! Kudos for introducing me to the project 💯

And for noticing this. It's my fault. Seems I updated the interfaces earlier in the day and forgot to regenerate the schema. That's usually something that's obvious in tests but in this case I changed something that didn't have any effect. I must think if there's another hook to force this test however it does run automatically before publishing.

Ah, that makes sense!

[gadicc]

🎉 This PR is included in version 1.5.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀