SWI-11043 Update Test Suite (#206)

* update current tests

* add script

* first batch

* oneOf example

* add new files

* add md guide

* rubocop

* update tn lookup

* update coverage

* update tfv

* fix missed tests

* update rubocop

* update simplecov

* add oauth test

* simplecov

* spelling

* more oauth tests

Author: ckoegel

.rubocop.yml

@@ -1,7 +1,7 @@
 # This file is based on https://github.com/rails/rails/blob/master/.rubocop.yml (MIT license)
 # Automatically generated by OpenAPI Generator (https://openapi-generator.tech)
 AllCops:
-  TargetRubyVersion: 2.4
+  TargetRubyVersion: 3.3
   # RuboCop has a bunch of cops enabled by default. This setting tells RuboCop
   # to ignore them, so only the ones explicitly set in this file are enabled.
   DisabledByDefault: true

Gemfile

@@ -5,6 +5,6 @@ gemspec
 group :development, :test do
   gem 'rake', '~> 13.2.1'
   gem 'pry-byebug'
-  gem 'rubocop', '~> 1.82.0'
-  gem 'simplecov', '~> 0.21.2'
+  gem 'rubocop', '~> 1.86.2'
+  gem 'simplecov', '~> 0.22.0'
 end

UPDATING_MODEL_TESTS.md

@@ -0,0 +1,319 @@
+# Updating Model Unit Tests
+
+## Context
+
+Model unit test files in `spec/unit/models/` are auto-generated by the `openapi-generator-cli` (`rake generate` or equivalent). The generator output is a *starting point* — every spec must be updated to match the template described here.
+
+This doc is the source of truth for that update step. Follow it exactly when bringing a fresh-generated spec into compliance.
+
+## Workflow
+
+1. Auto-generate the SDK (writes specs to `spec/unit/models/`).
+2. For each spec under `spec/unit/models/`, pick the matching template:
+   - [Standard Model Template](#standard-model-template) — regular models (the model file defines a `class`)
+   - [Enum Model Template](#enum-model-template) — pure enum models (the model file defines a class with `all_vars` and frozen string constants only)
+   - [oneOf Model Template](#oneof-model-template) — discriminator-based oneOf models (the model file defines a `module`, not a `class`)
+3. Run `rake unit` and confirm `0 failures`.
+4. Check coverage didn't regress meaningfully (see [Coverage notes](#coverage-notes)).
+
+## Standard Model Template
+
+Apply to any spec representing a regular model (object with attributes), e.g. `message_spec.rb`, `call_state_spec.rb`, `verify_code_request_spec.rb`.
+
+### Required structure
+
+```ruby
+describe Bandwidth::ModelName do
+  let(:model_name_default) { Bandwidth::ModelName.new }
+  let(:model_name_values)  { Bandwidth::ModelName.new({ <all attrs populated> }) }
+
+  describe '#initialize' do
+    it 'causes an ArgumentError by passing an Array to the initialize method'
+    it 'causes an ArgumentError by passing an invalid attribute to the initialize method'
+  end
+
+  describe '#acceptable_attributes' do
+    it 'expects acceptable JSON attributes to be those in the attribute map'
+  end
+
+  describe '#openapi_nullable' do
+    # If model has nullable attrs, assert the exact Set. Otherwise assert empty Set.
+    it 'expects nullable attributes to be ...'
+  end
+
+  describe '#build_from_hash' do
+    # camelCase keys in, snake_case attr reads out, instance check.
+    it 'validates instance of ModelName created by the build_from_hash method'
+  end
+
+  describe '#to_s' do
+    # Use the *populated* values instance, not the empty default.
+    it 'returns a string representation of the object'
+  end
+
+  describe '#eq? #==' do
+    # Assert both equality AND inequality.
+    it 'returns true/false when comparing objects'
+  end
+
+  describe '#to_body #to_hash' do
+    it 'returns a hash representation of the object'
+  end
+
+  # Only when the model has validated attrs (nil/pattern/min/max).
+  describe 'custom attribute writers' do
+    it '#attr=' do ... end
+  end
+end
+```
+
+### What to drop from the auto-generated spec
+
+- **`#hash` block.** Asserting `obj.hash.is_a?(Integer)` is testing Ruby itself. No signal.
+- **Empty `enum validation` block** (e.g. `describe 'enum validation' do; it 'works' do; end; end`). Pure noise — passes while testing nothing.
+- **`EnumAttributeValidator` block.** It tests generator scaffolding with fake allow-lists (`['valid']`, `[1]`), not the model's actual enums. The customer-facing behavior — "does an invalid enum value raise when set on a model?" — belongs in custom attribute writer tests, where it actually exercises the model.
+
+### What to add when missing
+
+- **`_default` let.** Pair `_values` with a default instance so `#eq?` can assert both equal-to-equal and equal-to-different. For models with required (non-nil-validated) attrs, build `_default` with only the required attrs populated.
+- **`#openapi_nullable` block.** Always include, even if the nullable set is empty. Read the model's `self.openapi_nullable` to get the exact set.
+- **Custom attribute writer tests.** One `it` per validated attribute. Cover every validation the model performs:
+  - `nil` check → `'<attr> cannot be nil'`
+  - pattern → `'invalid value for "<attr>", must conform to the pattern ...'`
+  - min/max length → `'invalid value for "<attr>", the character length must be ...'`
+  - min/max value → `'invalid value for "<attr>", must be smaller/greater than or equal to ...'`
+  - enum allow-list → `'invalid value for "<attr>", must be one of ...'`
+
+### What to fix when incorrect
+
+- **`#to_s` using the empty `_default` instance.** Switch to `_values` so the assertion is meaningful (asserts `'{}'` proves nothing).
+- **`#eq?` only checking equality.** Update to assert both `_default.eql?(equal_instance)` is true AND `_default.eql?(_values)` is false.
+
+## Enum Model Template
+
+Apply to pure enum models, e.g. `call_state_enum_spec.rb`.
+
+```ruby
+describe Bandwidth::SomeEnum do
+  describe 'constants' do
+    it 'defines VALUE_NAME' do
+      expect(Bandwidth::SomeEnum::VALUE_NAME).to eq('value_string')
+    end
+    # ...one per enum constant
+  end
+
+  describe '.all_vars' do
+    it 'returns every valid enum value' do
+      expect(Bandwidth::SomeEnum.all_vars).to eq([<every value in order>])
+    end
+  end
+
+  describe '.build_from_hash' do
+    it 'returns the value when it matches a valid enum value' do
+      expect(Bandwidth::SomeEnum.build_from_hash('valid_value')).to eq('valid_value')
+      # ...one per valid value
+    end
+
+    it 'raises an error for an invalid enum value' do
+      expect { Bandwidth::SomeEnum.build_from_hash('invalid') }.to raise_error(RuntimeError)
+    end
+  end
+end
+```
+
+### What to drop
+
+- Instance-construction tests (`Bandwidth::SomeEnum.new` then `be_instance_of`). The enum module isn't meant to be instantiated for use — only `build_from_hash` matters.
+
+## oneOf Model Template
+
+Apply to discriminator-based oneOf models, e.g. `callback_spec.rb`. These models are defined as Ruby `module`s (not classes) and act as routers — given a payload with a discriminator field, they dispatch to one of several target classes via `.build`.
+
+You can identify a oneOf model by inspecting `lib/bandwidth-sdk/models/<name>.rb`: it will be a `module` exposing `openapi_one_of`, `openapi_discriminator_name`, `openapi_discriminator_mapping`, and `build` as class methods.
+
+```ruby
+describe Bandwidth::SomeOneOf do
+  describe '.openapi_one_of' do
+    it 'lists the classes defined in oneOf' do
+      expect(Bandwidth::SomeOneOf.openapi_one_of).to eq([
+        :'ClassA',
+        :'ClassB'
+      ])
+    end
+  end
+
+  describe '.openapi_discriminator_name' do
+    it 'returns the discriminator property name' do
+      expect(Bandwidth::SomeOneOf.openapi_discriminator_name).to eq(:'type')
+    end
+  end
+
+  describe '.openapi_discriminator_mapping' do
+    it 'maps every discriminator value to a oneOf class' do
+      expect(Bandwidth::SomeOneOf.openapi_discriminator_mapping).to eq({
+        :'discriminator-value-1' => :'ClassA',
+        :'discriminator-value-2' => :'ClassB'
+      })
+    end
+
+    it 'maps only to classes listed in openapi_one_of' do
+      mapping_targets = Bandwidth::SomeOneOf.openapi_discriminator_mapping.values.uniq.sort
+      expect(mapping_targets).to eq(Bandwidth::SomeOneOf.openapi_one_of.sort)
+    end
+  end
+
+  describe '.build' do
+    # Stub build_from_hash on each target class so we test the routing without
+    # depending on the target's construction logic (which is covered by its own spec).
+    it 'routes <ClassA> discriminator values to ClassA.build_from_hash' do
+      Bandwidth::SomeOneOf.openapi_discriminator_mapping.each do |discriminator, klass|
+        next unless klass == :'ClassA'
+        data = { type: discriminator.to_s }
+        expect(Bandwidth::ClassA).to receive(:build_from_hash).with(data).and_return(:class_a_result)
+        expect(Bandwidth::SomeOneOf.build(data)).to eq(:class_a_result)
+      end
+    end
+
+    # ...one block per oneOf target class
+
+    it 'returns nil when the discriminator value is missing' do
+      expect(Bandwidth::SomeOneOf.build({})).to be_nil
+    end
+
+    it 'returns nil when the discriminator value does not match any mapping' do
+      expect(Bandwidth::SomeOneOf.build({ type: 'unknown' })).to be_nil
+    end
+  end
+end
+```
+
+### Required structure
+
+- `.openapi_one_of` — assert the exact list of target class symbols
+- `.openapi_discriminator_name` — assert the exact discriminator property symbol
+- `.openapi_discriminator_mapping` — assert the exact mapping AND assert it only maps to classes in `openapi_one_of`
+- `.build` — one block per target class verifying routing (via stubbed `build_from_hash`), plus the two nil cases (missing discriminator, unknown discriminator value)
+
+### What to drop from the auto-generated spec
+
+- **Weak `not_to be_empty` assertions.** Replace with exact equality checks against the model's actual values.
+- **`mapping.values.sort == one_of.sort` assertion.** This is buggy in the generator output — `mapping.values` has duplicates when multiple discriminator values route to the same class. Use `.uniq.sort` instead.
+- **Empty `.build` describe block.** Replace with the stub-based routing tests above.
+
+### Why stub `build_from_hash`?
+
+oneOf targets often have many required attributes (and nested required models), making it tedious to construct valid data for every discriminator value. Stubbing isolates the test to what `.build` is actually doing — routing based on a discriminator. The target class's `build_from_hash` is covered by that class's own spec.
+
+### What to skip
+
+- `#initialize`, `#openapi_nullable`, `#to_s`, `#eq?`, `#to_body`, custom attribute writers — these don't apply. The oneOf model is a `module`, not an instantiable class.
+
+### Variant: discriminator-based `openapi_any_of`
+
+Some modules expose `openapi_any_of` instead of `openapi_one_of` but otherwise have the identical shape (same `openapi_discriminator_name`, `openapi_discriminator_mapping`, and `build` implementation). Treat them exactly like the oneOf template above — just substitute `openapi_any_of` for `openapi_one_of` everywhere (describe block name, assertion target, the `uniq.sort` cross-check).
+
+Example: `multi_channel_action_spec.rb` (the model is `lib/bandwidth-sdk/models/multi_channel_action.rb`).
+
+### Variant: no-discriminator `openapi_one_of`
+
+Some `oneOf` modules expose `openapi_one_of` but **no** discriminator (no `openapi_discriminator_name`/`openapi_discriminator_mapping`). Instead, `build(data)` iterates `openapi_one_of` and uses a private `find_and_cast_into_type` helper to attempt deserialization into each target class in order, returning the first match (or `nil` if none match).
+
+You **cannot use the stubbed-routing pattern** for these — `find_and_cast_into_type` does a pre-flight check against the target's `acceptable_attributes` before calling `build_from_hash`, so a naked stub of `build_from_hash` is bypassed. Use real payloads instead, sized to match one target's attribute shape.
+
+```ruby
+describe Bandwidth::SomeNoDiscriminatorOneOf do
+  describe '.openapi_one_of' do
+    it 'lists the classes defined in oneOf' do
+      expect(Bandwidth::SomeNoDiscriminatorOneOf.openapi_one_of).to eq([
+        :'ClassA',
+        :'ClassB'
+      ])
+    end
+  end
+
+  describe '.build' do
+    it 'routes payloads matching ClassA attributes to ClassA' do
+      data = { someClassAAttr: 'value' }
+      expect(Bandwidth::SomeNoDiscriminatorOneOf.build(data)).to be_instance_of(Bandwidth::ClassA)
+    end
+
+    it 'routes payloads matching ClassB attributes to ClassB' do
+      data = { someClassBAttr: 'value' }
+      expect(Bandwidth::SomeNoDiscriminatorOneOf.build(data)).to be_instance_of(Bandwidth::ClassB)
+    end
+
+    it 'returns nil when the payload does not match any oneOf schema' do
+      expect(Bandwidth::SomeNoDiscriminatorOneOf.build({ unknown: 'value' })).to be_nil
+    end
+  end
+end
+```
+
+Pick payload attributes that exist on only one target so `find_and_cast_into_type`'s `acceptable_attributes` check disambiguates correctly. Skip `.openapi_discriminator_name` and `.openapi_discriminator_mapping` blocks — those methods don't exist on these modules.
+
+Example: `rbm_message_content_rich_card_spec.rb`.
+
+## Gotchas
+
+### Writers unreachable through `Model.new`
+
+Some validated setters (notably `to=` and `media=` on `Bandwidth::Message`) only get invoked from `initialize` when the input is an Array. Passing `nil` to `Model.new` won't trigger the writer's nil check.
+
+**Fix:** call the writer directly on an instance.
+
+```ruby
+# Wrong — won't raise:
+expect { Bandwidth::Message.new({ to: nil }) }.to raise_error(ArgumentError, 'to cannot be nil')
+
+# Right — exercises the writer:
+expect { message_values.to = nil }.to raise_error(ArgumentError, 'to cannot be nil')
+```
+
+When in doubt, read the model's `initialize` and check whether `self.<attr> =` is gated behind a type check.
+
+### `#build_from_hash` needs full required-attr data for nested models
+
+When a model has an attribute typed as another model (e.g., `:'business_address' => :'Address'`), `build_from_hash` recursively deserializes that nested hash into a real `Address` instance. That instance's `initialize` runs all required-attr setters, so any missing required attr raises `ArgumentError`.
+
+**This bites only in the `#build_from_hash` test, not `#to_s` / `#to_body` / `_values`** — when you pass a nested hash to `Model.new` directly (via `_values`), the parent's `attr_accessor` just stores the hash verbatim; no recursive deserialization happens.
+
+```ruby
+# Wrong — raises ArgumentError: 'addr1 cannot be nil' inside Address.new:
+Bandwidth::VerificationRequest.build_from_hash({
+  businessAddress: { name: 'Bandwidth' },  # Address requires addr1, city, state, zip, url
+  # ...
+})
+
+# Right — full required-attr data for the nested Address:
+Bandwidth::VerificationRequest.build_from_hash({
+  businessAddress: { name: 'Bandwidth', addr1: '900 Main Campus Dr', city: 'Raleigh', state: 'NC', zip: '27606', url: 'https://www.bandwidth.com' },
+  # ...
+})
+```
+
+Apply this recursively — if the nested model itself has nested models with required attrs, fill those too. Check the nested model's setters (`def <attr>=` blocks raising `'<attr> cannot be nil'`) to know which attrs are required.
+
+### `#to_s` is sensitive to attribute order
+
+The expected string must match the exact order of attributes as serialized by `to_hash`. Generate the expected string by running the populated instance through `to_s` in a console and pasting the result. Don't hand-craft it.
+
+### Nullable attribute symbols
+
+`openapi_nullable` returns a `Set` of `Symbol`s using snake_case attribute names with a leading colon-quote (e.g. `:'parent_call_id'`). Mirror the model's `openapi_nullable` definition exactly.
+
+## Coverage notes
+
+Dropping `EnumAttributeValidator` blocks reduces line coverage slightly, because the nested `EnumAttributeValidator` class isn't exercised elsewhere. Two options:
+
+1. **Accept the dip.** That code is generator scaffolding; coverage of it is low-value.
+2. **Add an enum-validated writer test** for at least one attribute per model with enums (e.g. `Bandwidth::Message.new(direction: 'invalid')` should raise). This exercises `EnumAttributeValidator` through a real path and recovers the coverage.
+
+The coverage logic itself is being updated as part of expanding to all models — don't fight stale thresholds.
+
+## Verification
+
+```sh
+rake unit
+```
+
+Confirm `0 failures` before considering a spec done.

custom_templates/Gemfile.mustache

@@ -5,6 +5,6 @@ gemspec
 group :development, :test do
   gem 'rake', '~> 13.2.1'
   gem 'pry-byebug'
-  gem 'rubocop', '~> 1.82.0'
-  gem 'simplecov', '~> 0.21.2'
+  gem 'rubocop', '~> 1.86.2'
+  gem 'simplecov', '~> 0.22.0'
 end

generate-model-tests.sh

@@ -0,0 +1,16 @@
+#!/bin/bash
+
+# Generates new test files for models. Run from the root.
+
+# allow generator to write test files
+sed -i.bak 's|^/spec/\*\*|# /spec/**|' .openapi-generator-ignore && rm .openapi-generator-ignore.bak
+# remove current test files for models
+rm -f ./spec/unit/models/*
+# generate new test files for models
+openapi-generator-cli generate -i bandwidth.yml -o ./ -c openapi-config.yml -g ruby > /dev/null
+# move generated model test files to the correct location (exclude api tests)
+mv ./spec/models/* ./spec/unit/models/
+# remove remaining generated test files (api tests, etc.)
+rm -rf ./spec/api ./spec/models
+# discard changes to modified files only (leaves deletions and new test files intact)
+modified=$(git diff --name-only --diff-filter=M) && [ -n "$modified" ] && echo "$modified" | xargs git checkout --

lib/bandwidth-sdk/configuration.rb

@@ -217,6 +217,7 @@ def initialize
         body = JSON.parse(response.body)
         @access_token = body['access_token']
         @access_token_expiration = Time.now + body['expires_in']
+        @access_token
       }
 
       yield(self) if block_given?