CI: Test for the health of all code base URLs #2127

fallwith · 2023-07-11T04:03:22Z

Add a new unit test class for testing the health of all URLs defined in the code base. See the comments in test/new_relic/healthy_urls_test.rb for more details
Update ci_cron.yml to set a CI_CRON env var to true
Update .ruboycop.yml to permit throw and catch calls without parens
Add new skip_unless_ci_cron helper to permit tests to only run via ci_cron.yml
Add new skip_unless_newest_ruby helper to permit tests to only run on the newest Ruby
Add new agent_root helper to permit tests to quickly get to the agent root
Fix 5 URLs that the new test caught

resolves #2126

- Add a new unit test class for testing the health of all URLs defined in the code base. See the comments in `test/new_relic/healthy_urls_test.rb` for more details - Update `ci_cron.yml` to set a `CI_CRON` env var to true - Update `.ruboycop.yml` to permit `throw` and `catch` calls without parens - Add new `skip_unless_ci_cron` helper to permit tests to only run via `ci_cron.yml` - Add new `skip_unless_newest_ruby` helper to permit tests to only run on the newest Ruby - Add new `agent_root` helper to permit tests to quickly get to the agent root - Fix 5 URLs that the new test caught resolves #2126

Prevent the `env` tests from running the URL health check tests

always compare gem version objects

fallwith · 2023-07-11T05:44:00Z

The new test will only run via CI cron, and only on the newest Ruby version available.

Using a branch with a single bad URL, here is a demonstration: https://github.com/newrelic/newrelic-ruby-agent/actions/runs/5516187929

we only need to break out of the current loop, so throw/catch is overkill

github-actions · 2023-07-11T17:05:28Z

SimpleCov Report

	Coverage	Threshold
Line	94.25%	94%
Branch	85.75%	85%

kaylareopelle

I really like this test and the new helpers you've added. Approved as-is. Comments are more out of curiosity rather than a need for anything to change.

kaylareopelle · 2023-07-11T19:57:29Z

lib/new_relic/agent/instrumentation/memcache.rb

@@ -6,7 +6,7 @@
 # each with slightly different APIs and semantics.
 # See:
 #     http://www.deveiate.org/code/Ruby-MemCache/ (Gem: Ruby-MemCache)
-#     http://seattlerb.rubyforge.org/memcache-client/ (Gem: memcache-client)
+#     https://github.com/mperham/memcache-client (Gem: memcache-client)


Didn't know he wrote memcache-client!

Yeah, I'd be interested in knowing more about the history there. I think that repo is a fork of another fork of the original RubyForge code (which Mike also has commit access to). For whatever reason, it's what's linked to from the Rubygems.org page's "Homepage" link, so I'm using it here.

test/helpers/misc.rb

kaylareopelle · 2023-07-11T20:05:11Z

test/new_relic/healthy_urls_test.rb

+class HealthyUrlsTest < Minitest::Test
+  ROOT = File.expand_path('../../..', __FILE__).freeze
+  FILENAMES = %w[
+    baselines


I don't think I'm familiar with this file. Where is it and what does it do?

It's at test/performance/script/baselines. I haven't checked, but possibly it's used to power the perf tests' setting of a baseline from which a subsequent set of results is compared to.

kaylareopelle · 2023-07-11T20:06:06Z

test/new_relic/healthy_urls_test.rb

+    newrelic
+    newrelic_cmd
+    nrdebug


Oh, are these executables as well as files?

Yeah, these ones are really just Ruby scripts so they're eligible for having URLs in the source code. For now I went with an allowlist approach of filenames and extensions for specifying what to scan for URLs. In the future we may want to reverse this and have a blocklist of things like image files or perhaps even take a quick peek at the file's encoding and make a judgment call from that.

kaylareopelle · 2023-07-11T20:06:30Z

test/new_relic/healthy_urls_test.rb

+    js
+    json
+    md
+    proto


Can we make changes to the proto files?

Yes! Not only can we, but we have, and with a URL in fact! There is currently only 1 file at infinite_tracing/lib/proto/infinite_tracing.proto. That file has a URL in the source code comments at the top.

olleolleolle · 2023-07-12T05:59:48Z

Cool that you built this.

olleolleolle · 2023-07-12T06:30:15Z

test/new_relic/healthy_urls_test.rb

+  end
+
+  def gather_urls
+    Dir.glob(File.join(ROOT, '**', '*')).each_with_object({}) do |file, urls|


Minor: Can default the urls keys to be an empty array, ready to be << onto!

Example from docs: https://rubyapi.org/3.2/o/hash
h = Hash.new { |hash, key| "Default value for #{key}" }

Change we can do:
each_with_object(Hash.new { |h, k| h[k] = [] }).

Which then would mean that urls[url] ||= [] could be omitted in line 155

Thanks, @olleolleolle! Implemented with #2132.

Using Hash.new([]) in conjunction with += [] from Ulysee's blog post looks pretty slick too, but the Ruby docs example approach is more readable and predictable in its behavior, so we'll go with it.

LETTERS = ('a'..'z').to_a hash1 = LETTERS.each_with_object(Hash.new { |h, k| h[k] = [] }) { |l, h| h[l] << true } hash2 = LETTERS.each_with_object(Hash.new([])) { |l, h| h[l] += [true] } hash1 == hash2

Implement the [suggested improvement](#2127 (review)) for the URLs hash to have an empty files array for the hash value by default. Update an old library website to use RubyGems.org instead.

fallwith requested review from hannahramadan, kaylareopelle and tannalynn as code owners July 11, 2023 04:03

fallwith added 2 commits July 10, 2023 21:59

CI: don't run URL health checks via env tests

d101509

Prevent the `env` tests from running the URL health check tests

CI: fix newest_ruby helper

0be5af5

always compare gem version objects

CI: url checker - use 'break'

76c95a8

we only need to break out of the current loop, so throw/catch is overkill

kaylareopelle reviewed Jul 11, 2023

View reviewed changes

kaylareopelle approved these changes Jul 11, 2023

View reviewed changes

fallwith merged commit 02bf110 into dev Jul 11, 2023

fallwith deleted the gene_chandler_1961 branch July 11, 2023 23:53

fallwith mentioned this pull request Jul 11, 2023

Link to github, not rubyforge [ci skip] #2121

Merged

7 tasks

olleolleolle reviewed Jul 12, 2023

View reviewed changes

fallwith mentioned this pull request Jul 12, 2023

CI: URL checker - simplify hash value vivification #2132

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI: Test for the health of all code base URLs #2127

CI: Test for the health of all code base URLs #2127

fallwith commented Jul 11, 2023

fallwith commented Jul 11, 2023

github-actions bot commented Jul 11, 2023

kaylareopelle left a comment

kaylareopelle Jul 11, 2023

fallwith Jul 11, 2023

kaylareopelle Jul 11, 2023

fallwith Jul 11, 2023

kaylareopelle Jul 11, 2023

fallwith Jul 11, 2023

kaylareopelle Jul 11, 2023

fallwith Jul 11, 2023

olleolleolle commented Jul 12, 2023

olleolleolle Jul 12, 2023

fallwith Jul 12, 2023

CI: Test for the health of all code base URLs #2127

CI: Test for the health of all code base URLs #2127

Conversation

fallwith commented Jul 11, 2023

fallwith commented Jul 11, 2023

github-actions bot commented Jul 11, 2023

SimpleCov Report

kaylareopelle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olleolleolle commented Jul 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment