Skip to content

Conversation

@alexskr
Copy link
Member

@alexskr alexskr commented Nov 7, 2025

Related Issue:

Summary

This PR focuses primarily on stabilizing and refactoring tests in test_ontology_submissions_controller, as part of ongoing work for the user-admin-endpoints feature.
It introduces more deterministic behavior, modernizes assertions to idiomatic Minitest style, and removes shared suite-level state that was causing nondeterministic failures.

Note: This is not comprehensive — only the most failure-prone areas were addressed in this pass. Additional test suites will be refactored in follow-up PRs.

Key Improvements

  • Sorted unordered structures before comparison.
  • Removed random sampling (.shuffle, .sample) in tests.
  • Added guards against background forking during test runs (ENV['RACK_ENV'] != 'test').
  • Replaced generic or Ruby-style assertions with Minitest idioms: assert_equal, assert_includes, assert_operator, refute_includes, etc.
  • Eliminated global (@@) variables and class-level state in favor of per-test setup and teardown.
  • Restored missing assertions and corrected expected status codes (e.g., 204 on DELETE).
  • Added minitest-bisect to the Gemfile to help locate order-dependent failures.

alexskr and others added 17 commits October 27, 2025 17:23
Feature: Ontology Admin Endpoints for Logs and Submission Management
…ests

Replace non-idiomatic assertions with Minitest equivalents:
- assert(x == y) -> assert_equal
- .eql?() -> assert_equal
- assert !x.empty? -> assert_operator x.length, :>, 0
- submissions.all? -> submissions.each for better error messages

Extract repeated contact validation into helper method to reduce duplication.

Fix intermittent test failures by sorting array keys before comparison to handle non-deterministic ordering.
refs #197
- Replace class variables (@@) with class instance variables using accessors
- Add attr_accessor definitions for shared test state
- Modernize assertions to Minitest style (assert_equal, assert_includes, assert_nil)
- Improve code formatting and readability
…dition

- Replace random .sample(3) with deterministic .first(3)
- Use dynamic assertion for keep_ids count instead of hardcoded value
- Capture process_id before use for clarity
- Simplify polling loop condition to check only for 'done' or 'errors'
- Clean up assertion message formatting for consistency
…n/deletion

- Replace `assert 201, ...` with `assert_equal 201, ...` to properly compare response status.
- Add GET checks after slice creation to confirm the new slice exists.
- Remove redundant manual reset of LinkedData.settings.enable_security.
- Update DELETE test to expect 204 (No Content) instead of 201.
- Add GET checks after deletion to verify the slice was actually removed.
- Add explicit flunk when status_payload contains 'errors'
  to prevent false positives from incomplete or invalid responses
- Ensure test fails immediately on backend error instead of silently skipping assertions
Restore LinkedData.settings.enable_slices to its original value after tests.
…com:ncbo/ontologies_api into refactor/tests-stability-and-minitest-style
… user helpers

This change refactors `TestOntologySubmissionsController` to eliminate
suite-level globals and class-variable state in favor of per-test setup
and shared helpers:

- Replace `before_suite` + `@@acronym`/`@@name`/`@@file_params` with
  per-test `setup` that generates a unique ontology acronym and creates
  an ontology administered by a shared test user.
- Introduce a suite-local `USERNAME = "test_user"` and use
  `ensure_user(USERNAME)` to guarantee the user exists without races.
- Normalize assertion style:
  - Use `assert_equal(expected, actual, message)` with parentheses
  - Remove `msg=` pseudo-named args
  - Add `.sort` where appropriate when comparing unordered keys
- Replace direct references to `@@acronym`/`@@name` with instance vars
  (`@acronym`, `@name`).

New helper methods (in the base test class) used by this suite:
- `create_user(username, email:, password:)` — always creates a user
- `ensure_user(username, email:, password:)` — idempotently finds or creates
- `delete_user(username)` — removes a user if present

Notes:
- `delete_ontologies_and_submissions` now runs in `setup` to guarantee
  a clean slate per test. `after_all` also cleans up the shared user via
  `delete_user(USERNAME)`. If the extra cleanup proves redundant, we can
  drop `before_all`/`after_all` and keep only the per-test cleanup.
@codecov-commenter
Copy link

codecov-commenter commented Nov 7, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.46%. Comparing base (558c168) to head (1392b88).

Files with missing lines Patch % Lines
controllers/ontology_submissions_controller.rb 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                       Coverage Diff                        @@
##           feature/user-admin-endpoints     #198      +/-   ##
================================================================
+ Coverage                         72.86%   73.46%   +0.60%     
================================================================
  Files                                63       63              
  Lines                              3376     3377       +1     
================================================================
+ Hits                               2460     2481      +21     
+ Misses                              916      896      -20     
Flag Coverage Δ
unittests 73.46% <66.66%> (+0.60%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@alexskr alexskr marked this pull request as ready for review November 7, 2025 18:02
@alexskr alexskr merged commit f115fbb into feature/user-admin-endpoints Nov 7, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants