review(#1154): answering to 20/11/2025 requested changes

Author: kennethrioja

lib/ingestors/github_ingestor.rb

@@ -23,7 +23,7 @@ class GithubIngestor < Ingestor # rubocop:disable Metrics/ClassLength
 
     GITHUB_API_BASE = 'https://api.github.com/repos'
     CACHE_PREFIX = 'github_ingestor_'
-    TTL = 1.week # time to live after the cache is deleted
+    TTL = 1.week # cache expiration time (time to live before cache expires)
 
     def self.config
       {
@@ -57,7 +57,7 @@ def read(source_url)
         add_material to_material(repo_data)
       end
     rescue StandardError => e
-      @messages << "#{self.class.name} read failed, #{e.message}"
+      Rails.logger.error("#{e.class}: read() failed, #{e.message}")
     end
 
     private
@@ -94,13 +94,9 @@ def github_api_from_io(uri, parts)
     # url: url to open
     # ttl: time-to-live in seconds (default 7 days)
     def cache_fetch(key, url)
-      Rails.cache.fetch(key, expires_in: TTL) do
+      Rails.cache.fetch(key, expires_in: TTL, skip_nil: true) do
         JSON.parse(open_url(url).read)
       end
-    rescue StandardError => e
-      @messages << "#{self.class.name} cache_fetch failed for #{url}, #{e.message}"
-      yield if block_given?
-      nil
     end
 
     # Sets material hash keys and values and add them to material
@@ -137,15 +133,15 @@ def fetch_homepage_doc(url)
       Nokogiri::HTML(response.body)
     end
 
-    # DEFINITION – Opens the GitHub homepage, fetches the 3 first >25 char <p> tags'text
+    # DEFINITION – Opens the GitHub homepage, fetches the 3 first >50 char <p> tags'text
     # and joins them with a 'Read more...' link at the end of the description
     # Some of the first <p> tags were not descriptive, thus skipping them
     def fetch_definition(doc, url)
       desc = ''
       round = 3
       doc.css('p').each do |p|
-        p_txt = p&.text&.strip&.gsub(/\s+/, ' ')
-        next if (p_txt.length < 50) || round.zero?
+        p_txt = p&.text&.strip&.gsub(/\s+/, ' ') || ''
+        next if p_txt.length < 50 || round.zero?
 
         desc = "#{desc}\n#{p_txt}"
         round -= 1
@@ -187,6 +183,8 @@ def fetch_latest_release(full_name)
     # CONTRIBUTORS – Opens contributors API address and returns list of contributors
     def fetch_contributors(contributors_url, full_name)
       contributors = cache_fetch("#{CACHE_PREFIX}contributors_#{full_name.gsub('/', '_')}", contributors_url)
+      return [] unless contributors
+
       contributors.map { |c| (c['login']) }
     end
 
@@ -200,7 +198,7 @@ def fetch_prerequisites(doc)
       # ... any tag with id containing "prereq" (EN) or "prerreq" (ES)
       prereq_paragraphs = fetch_prerequisites_from_id_or_class(doc, prereq_paragraphs) if prereq_paragraphs.empty?
 
-      prereq_paragraphs&.join("\n")&.gsub(/\n\n+/, "\n")&.to_s&.strip
+      prereq_paragraphs&.join("\n")&.gsub(/\n\n+/, "\n")&.strip || ''
     end
 
     def fetch_prerequisites_from_h(doc, prereq_paragraphs)

lib/ingestors/ingestor.rb

@@ -78,7 +78,7 @@ def get_redirected_url(url, limit = 5) # rubocop:disable Metrics/AbcSize
       raise 'Too many redirects' if limit.zero?
 
       https_url = to_https(url) # some `homepage` were http
-      response = HTTParty.get(https_url, follow_redirects: true, headers: { 'User-Agent' => config[:user_agent] })
+      response = HTTParty.get(https_url, follow_redirects: true, headers: { 'User-Agent' => config[:user_agent] || 'TeSS Bot' })
       return https_url unless response.headers['content-type']&.include?('html')
 
       doc = Nokogiri::HTML(response.body)

test/unit/ingestors/github_ingestor_test.rb

@@ -21,7 +21,6 @@ def initialize(ingestor)
           decorator_ref.open_url_counter + 1
         )
 
-        # puts("open_url has been used, total #{decorator_ref.open_url_counter}")
         super(url, **kwargs)
       end
     end
@@ -157,7 +156,7 @@ def materials
       ingestor.open_url("| touch #{file}")
     rescue StandardError
     end
-    `ls #{dir}` # This is needed or the `exist?` check below seems to return a stale result
+    Dir.children(dir) # This is needed or the `exist?` check below seems to return a stale result
     refute File.exist?(file)
   end
 
@@ -322,17 +321,17 @@ def materials
   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(:get_sources, ->(*) { raise StandardError, 'test failure' }) do
+      mock_logger = Minitest::Mock.new
+      mock_logger.expect(:error, nil, ['StandardError: read() failed, test failure'])
 
-    @ingestor.stub(:open_url, ->(_url) { raise StandardError, 'test failure' }) do
-      @ingestor.send(:cache_fetch, 'key', 'https://github.com/example')
+      Rails.stub(:logger, mock_logger) do
+        @ingestor.send(:read, 'https://github.com/example')
+      end
+      mock_logger.verify
     end
-    assert_includes @ingestor.instance_variable_get(:@messages).last,
-                    'Ingestors::GithubIngestor cache_fetch failed for https://github.com/example, test failure'
+
+    assert true, 'Mock verification passed'
   end
 
   test 'prereq_node? returns true when id includes prereq' do