[Chore] Increase test coverage

[malparty]

Issue

Automated tests do not cover the core business logic (CSV upload, scraping).

Expected

While 100% test coverage is not required for this code challenge, all critical paths of the application should be unit-tested (and, ideally, UI-tested). So the following should be covered:

• CSV parsing
• Keyword creation
• Keyword scraping
• Keyword listing

[!NOTE] There is a bit of a challenge to implement tests for a service that generates HTTP requests. Hence, it is an expected part of the submission for us to assess your skills in writing challenging tests.

[macropusgiganteus]

Thank you for your feedback.

I understand that the core logic should be covered in test.

If I were able to add tests, I would have done the following:

CSV parsing As we discussed in #10, we can refactor the code and move CSV parsing related logic into a separate object. I would create a CSV service object that contains following methods

• initialize: takes file_path then set it as the property @file_path
• read_file: return rows if CSV.read() success else []
  • [test] read file when the file exists. Expect rows to be returned.
  • [test] read file when the file doesn't exist. Expect [] to be returned.
• count_rows: return rows count
  • [test] count rows. Expect correct rows count (from a test CSV file) to be returned.
• parse_objects: read a file and parse rows to array of Keyword
  • [test] parse object. Expect an array of Keyword to be returned.

** CSV files for test will be at test/fixtures

Keyword creation I would create another function process_keyword that meant process a single keyword. Then I would call multiple process_keyword inside a parent function (currently import_csv) Test process_keyword

1. Test that a keyword is persisted to DB.
2. Mock scraping result.
3. Test that a keyword result is persisted to DB.

Keyword scraping Scraping can be split into 2 parts

1. HTTP client I would mock the http response using webmock and use httparty as the http client.
2. HTML parsing I would create a service object for HTML parsing that contains following methods:
   • initialize: takes response.body then use Nokogiri::HTML() to parse it into HTML document and set property @document.
   • extract_ads: return CSS selector result for ads element
     • [test] test return the elements when exist
     • [test] test return nil when the elements doesn't exist
   • extract_links return CSS selector result for link element
     • [test] test return the elements when exist
     • [test] test return nil when the elements doesn't exist
   • extract_search_results return CSS selector result for search result stats
     • [test] test return the elements when exist
     • [test] test return nil when the elements doesn't exist

Keyword listing Some are covered in (test/keywords) test/controllers/keywords_controller_test.rb I would add more assertions to ensure that each column of a row contains the correct data matching the test fixtures.

[malparty]

Thanks for your detailed answer 🙏