test(github-ingestor): all related code is 100% covered

Author: kennethrioja

lib/ingestors/github_ingestor.rb

@@ -55,9 +55,9 @@ def read(source_url)
 
         # Add to material
         add_material to_material(repo_data)
-      rescue StandardError => e
-        @messages << "#{self.class.name} read failed for #{url}, #{e.message}"
       end
+    rescue StandardError => e
+      @messages << "#{self.class.name} read failed, #{e.message}"
     end
 
     private
@@ -92,26 +92,14 @@ def github_api_from_com(uri)
     # One GitHub URL equals to 4 GitHub API requests.
     # key: string key for the cache
     # ttl: time-to-live in seconds (default 7 days)
-    def get_or_set_cache(key, url) # rubocop:disable Metrics/AbcSize
-      content = open_url(url)
-      data = content ? JSON.parse(content.read) : nil
-
-      set_cache(key, data) if Rails.cache.read(key).nil? # sets cache only if there is no cache yet or is expired
-      get_cache(key)
-      data
+    def get_or_set_cache(key, url)
+      Rails.cache.fetch(key, expires_in: TTL) do
+        JSON.parse(open_url(url).read)
+      end
     rescue StandardError => e
       @messages << "#{self.class.name} get_or_set_cache failed for #{url}, #{e.message}"
-      yield
-    end
-
-    def set_cache(key, data, ttl: TTL)
-      Rails.logger.info "[Github Cache] SET cache #{key}"
-      Rails.cache.write(key, data, expires_in: ttl) unless data.nil?
-    end
-
-    def get_cache(key)
-      Rails.logger.info "[Github Cache] GET cache #{key}"
-      Rails.cache.read(key)
+      yield if block_given?
+      nil
     end
 
     # Sets material hash keys and values and add them to material

test/fixtures/files/ingestion/github/mock.html

@@ -9,5 +9,4 @@
     <h1>Prerequisites</h1>
       <p>1. Be kind</p>
   </body>
-</html>
-Week 4: Packaging and Quality Control
+</html>

test/helpers/sitemap_helper_test.rb

@@ -0,0 +1,53 @@
+require 'test_helper'
+
+class SitemapHelperTest < ActionView::TestCase
+  setup do
+    @ingestor = DummyIngestor.new
+    @messages = []
+  end
+
+  test 'get_sources with url only returns url only' do
+    url = 'https://test.com'
+    assert_equal @ingestor.send(:get_sources, url), [url]
+  end
+
+  test 'get_sources returns parsed URLs from sitemap.xml' do
+    sitemap_xml = <<~XML
+      <?xml version="1.0" encoding="UTF-8"?>
+      <urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
+        <url>
+          <loc>https://app.com/events/123</loc>
+        </url>
+        <url>
+          <loc>https://app.com/events/456</loc>
+        </url>
+      </urlset>
+    XML
+
+    stub_request(:get, 'https://app.com/events/sitemap.xml')
+      .to_return(status: 200, body: sitemap_xml, headers: { 'Content-Type' => 'application/xml' })
+
+    urls = @ingestor.send(:get_sources, 'https://app.com/events/sitemap.xml')
+    assert_equal ['https://app.com/events/123', 'https://app.com/events/456'], urls
+  end
+
+  test 'get_sources returns parsed URLs from sitemap.txt' do
+    sitemap_txt = <<~TXT
+      https://app.com/events/123
+      https://app.com/events/456
+    TXT
+
+    stub_request(:get, 'https://app.com/events/sitemap.txt')
+      .to_return(status: 200, body: sitemap_txt, headers: { 'Content-Type' => 'txt' })
+
+    urls = @ingestor.send(:get_sources, 'https://app.com/events/sitemap.txt')
+    assert_equal ['https://app.com/events/123', 'https://app.com/events/456'], urls
+  end
+
+  private
+
+  def webmock(url, filename)
+    file = Rails.root.join('test', 'fixtures', 'files', 'ingestion', filename)
+    WebMock.stub_request(:get, url).to_return(status: 200, headers: {}, body: file.read)
+  end
+end

test/test_helper.rb

@@ -400,6 +400,21 @@ def mock_facets
       end
     end
   end
+
+  class DummyIngestor < Ingestors::Ingestor
+    include Ingestors::Concerns::SitemapHelpers
+
+    def self.config
+      {
+        key: 'dummy'
+      }
+    end
+
+    def read(url)
+      get_sources(url)
+      open_url(url)
+    end
+  end
 end
 
 # Minitest's `stub` method but ignores any blocks

test/unit/ingestors/github_ingestor_test.rb

@@ -39,6 +39,10 @@ class GithubIngestorTest < ActiveSupport::TestCase # rubocop:disable Metrics/Cla
     webmock('https://api.github.com/repos/hsf-training/cpluspluscourse/releases', 'github/releases.json')
     webmock('https://api.github.com/repos/swcarpentry/python-novice-inflammation/releases', 'github/releases.json')
     webmock('https://api.github.com/repos/hsf-training/hsf-training-scikit-hep-webpage/releases', 'github/releases.json')
+
+    # HTML
+    html = File.read(Rails.root.join('test/fixtures/files/ingestion/github/mock.html'))
+    @doc = Nokogiri::HTML(html)
   end
 
   teardown do
@@ -235,6 +239,102 @@ class GithubIngestorTest < ActiveSupport::TestCase # rubocop:disable Metrics/Cla
     assert_nil sample_after_ttl_and_change.description
   end
 
+  test 'std errors when exception is raised' do
+    @ingestor.stub(:get_sources, ->(_url) { raise StandardError, 'test failure' }) do
+      @ingestor.send(:read, 'https://github.com/example')
+    end
+    assert_includes @ingestor.instance_variable_get(:@messages).last,
+                    'Ingestors::GithubIngestor read failed, test failure'
+
+    @ingestor.stub(:open_url, ->(_url) { raise StandardError, 'test failure' }) do
+      @ingestor.send(:get_or_set_cache, 'key', 'https://github.com/example')
+    end
+    assert_includes @ingestor.instance_variable_get(:@messages).last,
+                    'Ingestors::GithubIngestor get_or_set_cache failed for https://github.com/example, test failure'
+
+    @ingestor.stub(:get_or_set_cache, ->(_key, _url) { raise StandardError, 'test failure' }) do
+      @ingestor.send(:fetch_doi, 'full_name')
+    end
+    assert_includes @ingestor.instance_variable_get(:@messages).last,
+                    'Ingestors::GithubIngestor fetch_doi failed for https://api.github.com/repos/full_name/contents/README.md, test failure'
+
+    @ingestor.stub(:get_or_set_cache, ->(_key, _url) { raise StandardError, 'test failure' }) do
+      @ingestor.send(:fetch_latest_release, 'full_name')
+    end
+    assert_includes @ingestor.instance_variable_get(:@messages).last,
+                    'Ingestors::GithubIngestor fetch_latest_release failed for https://api.github.com/repos/full_name/releases, test failure'
+
+    @ingestor.stub(:get_or_set_cache, ->(_key, _url) { raise StandardError, 'test failure' }) do
+      @ingestor.send(:fetch_contributors, 'my.url', 'full_name')
+    end
+    assert_includes @ingestor.instance_variable_get(:@messages).last,
+                    'Ingestors::GithubIngestor fetch_contributors failed for my.url, test failure'
+  end
+
+  test 'prereq_node? returns true when id includes prereq' do
+    node = Nokogiri::XML::Node.new('div', @doc)
+    node['id'] = 'prerequisites'
+    assert @ingestor.send(:prereq_node?, node)
+  end
+
+  test 'prereq_node? returns true when class includes prereq' do
+    node = Nokogiri::XML::Node.new('div', @doc)
+    node['class'] = 'has_prereq_section'
+    assert @ingestor.send(:prereq_node?, node)
+  end
+
+  test 'prereq_node? returns false for unrelated id/class' do
+    node = Nokogiri::XML::Node.new('div', @doc)
+    node['id'] = 'requirements'
+    refute @ingestor.send(:prereq_node?, node)
+  end
+
+  test 'extract_following_paragraphs finds immediate sibling paragraphs' do
+    doc = Nokogiri::HTML(<<~HTML)
+      <div id="prereq"></div>
+      <p>First paragraph</p>
+      <ul><li>List item</li></ul>
+      <h2>Stop here</h2>
+    HTML
+
+    node = doc.at_xpath('//*[@id="prereq"]')
+    paragraphs = []
+    @ingestor.send(:extract_following_paragraphs, node, paragraphs)
+
+    assert_equal 2, paragraphs.size
+    assert_equal %w[p ul], paragraphs.map(&:name)
+  end
+
+  test 'extract_nested_paragraphs finds paragraphs inside the node' do
+    doc = Nokogiri::HTML(<<~HTML)
+      <div id="prereq">
+        <p>Nested paragraph</p>
+        <ul><li>Nested list</li></ul>
+      </div>
+    HTML
+
+    node = doc.at_xpath('//*[@id="prereq"]')
+    paragraphs = []
+    @ingestor.send(:extract_nested_paragraphs, node, paragraphs)
+
+    assert_equal 2, paragraphs.size
+    assert_equal %w[p ul], paragraphs.map(&:name)
+  end
+
+  test 'fetch_prerequisites_from_id_or_class collects prerequisites correctly' do
+    doc = Nokogiri::HTML(<<~HTML)
+      <div id="prereq"></div>
+      <p>Be kind</p>
+      <p>Have Ruby installed</p>
+    HTML
+
+    paragraphs = []
+    result = @ingestor.send(:fetch_prerequisites_from_id_or_class, doc, paragraphs)
+
+    assert_equal 2, result.size
+    assert_equal 'Be kind', result.first.text
+  end
+
   private
 
   def webmock(url, filename)

test/unit/ingestors/ingestor_test.rb

@@ -113,4 +113,78 @@ class IngestorTest < ActiveSupport::TestCase
     assert_nil(event.language)
   end
 
+  test 'open_url returns content when URL is valid' do
+    ingestor = DummyIngestor.new
+    stub_request(:get, 'https://example.com').to_return(body: 'ok', status: 200)
+
+    result = ingestor.open_url('https://example.com')
+    assert_equal 'ok', result.read
+  end
+
+  test 'open_url raises HTTPRedirect after too many retries' do
+    ingestor = DummyIngestor.new
+    fake_uri = URI('https://example.com/')
+
+    URI.stub(:parse, fake_uri) do
+      fake_uri.define_singleton_method(:open) do |*_args|
+        raise OpenURI::HTTPRedirect.new('Redirect', 1, URI('https://redirected.com'))
+      end
+
+      assert_raises(OpenURI::HTTPRedirect) do
+        ingestor.open_url('https://example.com/')
+      end
+    end
+  end
+
+  test 'open_url raises HTTPError' do
+    ingestor = DummyIngestor.new
+    stub_request(:get, 'https://bad.com')
+      .to_raise(OpenURI::HTTPError.new('404 not found', StringIO.new))
+
+    result = ingestor.open_url('https://bad.com')
+    assert_nil result
+    assert_includes ingestor.instance_variable_get(:@messages).last,
+                    "Couldn't open URL https://bad.com: 404 not found"
+  end
+
+  test 'get_redirected_url follows meta refresh redirect' do
+    ingestor = DummyIngestor.new
+    html_with_meta = '<html><head><meta http-equiv="Refresh" content="0; url=/redirected"></head></html>'
+    redirected_html = '<html><head><title>Final</title></head><body>Done</body></html>'
+
+    HTTParty.stub(:get, lambda { |url, **|
+      case url
+      when 'https://example.com/'
+        OpenStruct.new(headers: { 'content-type' => 'text/html' }, body: html_with_meta)
+      when 'https://example.com//redirected'
+        OpenStruct.new(headers: { 'content-type' => 'text/html' }, body: redirected_html)
+      else
+        raise "Unexpected URL: #{url}"
+      end
+    }) do
+      result = ingestor.get_redirected_url('http://example.com/')
+      assert_equal 'https://example.com//redirected', result
+    end
+  end
+
+  test 'get_redirected_url returns original url when no meta redirect' do
+    ingestor = DummyIngestor.new
+    html_no_meta = '<html><head><title>No redirect</title></head><body></body></html>'
+
+    HTTParty.stub(:get, ->(_url, **) { OpenStruct.new(headers: { 'content-type' => 'text/html' }, body: html_no_meta) }) do
+      result = ingestor.get_redirected_url('http://example.com/')
+      assert_equal 'https://example.com/', result
+    end
+  end
+
+  test 'get_redirected_url raises when too many redirects' do
+    ingestor = DummyIngestor.new
+    html_with_meta = '<html><head><meta http-equiv="Refresh" content="0; url=/loop"></head></html>'
+
+    HTTParty.stub(:get, ->(_url, **) { OpenStruct.new(headers: { 'content-type' => 'text/html' }, body: html_with_meta) }) do
+      assert_raises(RuntimeError, 'Too many redirects') do
+        ingestor.get_redirected_url('http://example.com/', 0)
+      end
+    end
+  end
 end