Test optimization#4897
Draft
johha wants to merge 12 commits into
Draft
Conversation
johha
force-pushed
the
test-optimization
branch
2 times, most recently
from
9f8ccf4 to
99b2c57
Compare
Member
|
I have some PRs that may be relevant to this: |
Contributor
Author
This PR is 'just' AI trying to find some optimizations. I might split it up once I'm satisfied with the results. |
johha
force-pushed
the
test-optimization
branch
2 times, most recently
from
ef56d24 to
cde4b81
Compare
This commit significantly improves test load times for specs that don't require full database initialization, reducing individual spec file load time from ~10s to ~0.6s (95% faster). - Added support for shared examples and custom matchers - Included RSpec::Its for one-liner attribute testing - Made it a drop-in replacement for spec_helper where database is not needed - New helper for blobstore specs that need clean Fog state between tests - Prevents test pollution from Fog's global state - Used by 10 blobstore-related job specs - Improved for specs needing database but not full Rails environment - Used by 24 specs (fetchers, decorators, presenters, repositories) These specs test pure Ruby logic (parameter validation, utilities, parsers): **Message specs (107 files):** - List message specs: apps, builds, deployments, domains, droplets, events, feature_flags, isolation_segments, metadata, orgs, packages, processes, roles, routes, security_groups, service_*, sidecars, spaces, stacks, tasks, users - Show message specs: app, domain, role, route, service_*, shared_spaces, space - Apply message specs: org_quota, space_quota, security_group - Update message specs: app_feature, deployment, domain, droplet, feature_flags, isolation_segment, org_default_iso_seg, package, process, revision, sidecar, space_feature, space_update_isolation_segment, update_environment_variables, user - Create message specs: buildpack, domain, droplet, isolation_segment, role, service_credential_binding, sidecar, stack, user - Delete message specs: domain_delete_shared_org, space_delete_unmapped_routes - Other: manifest_buildpack, manifest_process_update, manifest_service_binding, metadata_base, purge, quotas_*, to_many_relationship, v2_v3_resource_translator - Validators: url_validator, validators (base) - Mixins: authentication_message_mixin **Lib utility specs (38 files):** - cloud_controller/: adjective_noun_generator, blob_sender, byte_quantity, database_parts_parser, diego/* (lifecycle_data, docker_uri_converter, droplet_url_generator, failure_reason_sanitizer, ssh_key), file_path_checker, metrics/request_metrics, paging/pagination_options, random_route_generator, routing_api/*, secrets_fetcher, steno_configurer, telemetry_logger, url_secret_obfuscator - Top-level: http_*_error, index_stopper, structured_error, utils/uri_utils, vcap/digester, vcap/host_system - rest_controller/common_params **Presenter specs (3 files):** - v3/: cache_key, domain_shared_orgs, to_many_relationship - Fixed Config stub conflicts in distributed_scheduler_spec - Fixed instances_reporter to properly handle fog state - All converted specs pass with lightweight_spec_helper - RuboCop clean (no offenses) - Total: 109 specs now load in ~0.6s instead of ~10s - **70% of message specs** (107/152) now use lightweight_spec_helper - **13% of lib specs** (38/302) converted - **Significant developer productivity improvement** - faster test feedback loops - Remaining specs have legitimate dependencies on database/models/Rails - 109 spec files converted to lightweight_spec_helper - 24 spec files converted to db_spec_helper - 10 blobstore job specs converted to fog_spec_helper - 3 spec helper files enhanced/created
Extends the test optimization started in 09b3a83 by converting additional message and lib specs to lightweight_spec_helper, with explicit dependency requires per spec. Also adds rspec/its to the lightweight helper and stubs Config.config in build_create_message_spec to prevent test pollution when run alongside specs that load the real Config class.
Specs that operate on POROs/doubles rather than Sequel models can use the lightweight helper, cutting per-file load time from ~10s to <2s.
Middleware classes are POROs that operate on Rack envs; the specs already use doubles for almost everything, so they can run on the lightweight helper without touching the DB or Rails.
Service-broker error classes, schema validators, SSO command/differ classes, and the UAA client manager are POROs that don't need Rails or the DB.
- Add middleware/ to $LOAD_PATH so middleware specs can require their classes. - Make TestConfig stub conditional on defined?(TestConfig) — lightweight specs that don't transitively load TestConfig no longer crash. - Support nested-key access in StubConfig#get(*keys) to mirror the real Config#get behavior (e.g. Config.config.get(:db, :log_db_queries)).
- telemetry_logger.rb: require 'oj' (was relying on transitive load via spec_helper) - telemetry_logger_spec.rb: require missing 'vcap/request' and 'support/time_helpers' - lightweight_spec_helper.rb: stub Config.config when nil so list_message validations that read renderer.max_total_results don't NoMethodError when the real Config class is loaded (and clobbers the bootstrap stub) by another spec in the pool
stub_const(..., Class.new) creates an empty class, which trips verify_partial_doubles when other specs in the parallel CI run load spec_helper (which enables that flag globally for all tests). Stub a class that actually declares the method being mocked.
- Drop ActiveModel::Validations#errors_on extension from lightweight_spec_helper; revert specs using errors_on/error_on back to spec_helper instead of carrying a shim in the helper. - Remove fog_spec_helper.rb; revert fog-isolation specs back to spec_helper (Fog itself is being removed in the near term). - Revert specs that ended up with >=6 explicit requires after conversion (e.g. presenters/v2/default_presenter_spec) — at that point spec_helper is the cleaner choice. Lightweight pool drops from 241 -> 197 specs, but the helper stays focused and the reverted files are noticeably simpler.
- Style/IfUnlessModifier in lightweight_spec_helper.rb (collapse to modifier) - Lint/EmptyClass in bbs_task_client_spec.rb (use Class.new constant) - Style/MultipleComparison in api_presenter_spec.rb (use Array#include?)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks for contributing to cloud_controller_ng. To speed up the process of reviewing your pull request please provide us with:
A short explanation of the proposed change:
An explanation of the use cases your change solves
Links to any other associated PRs
I have reviewed the contributing guide
I have viewed, signed, and submitted the Contributor License Agreement
I have made this pull request to the
mainbranchI have run all the unit tests using
bundle exec rakeI have run CF Acceptance Tests