CMR-11368: Databine test improvements by jceaser · Pull Request #2472 · nasa/Common-Metadata-Repository

jceaser · 2026-07-29T19:36:07Z

Overview

Background

This ticket started out as a Snyk ticket, however Jackson had a lot of issues and another ticket ended up applying the needed library change. In the course of doing the work to support 2.21.5 severely intermittent test issues prevented this ticket from being properly tested in bamboo and it was decided to fix those issues.

What are the changes?

Moving two tests from access control app to system int test
adding serial to tests so that the tests in question would not run while other tests run
adding try/final blocks to ensure that when the service being tested is set to read only mode, it is always set back so that there are no cascading errors from these tests.

What areas of the application does this impact?

access-control-app
system-int-test

Required Checklist

New and existing unit and int tests pass locally and remotely
clj-kondo has been run locally and all errors in changed files are corrected
I have commented my code, particularly in hard-to-understand areas
[-] I have made changes to the documentation (if necessary)
[-] My changes generate no new warnings

codecov-commenter · 2026-07-29T21:50:54Z

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.94%. Comparing base (62070d1) to head (dd40499).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...cmr/access_control/services/acl_search_service.clj	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2472      +/-   ##
==========================================
- Coverage   57.96%   57.94%   -0.02%     
==========================================
  Files        1074     1074              
  Lines       74707    74710       +3     
  Branches     2189     2189              
==========================================
- Hits        43303    43293      -10     
- Misses      29355    29369      +14     
+ Partials     2049     2048       -1

☔ View full report in Codecov by Harness.
📢 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.

…ge write status better

…ests as serial

…ing the other testing threads

jceaser · 2026-09-02T18:44:22Z

-          (group-permission-parameter-subfield-validation params)
-          (group-permission-permission-validation params)))
+  (let [gp (:group-permission params)]
+    (if (and (some? gp) (not (map? gp)))


Sometimes, a null error would show up in tests

why would gp ever be null?

I have lost the original error, maybe this was a cast problem, I forget now, let me see if I can figure that out

This was an invalid cast that could come up if someone sent in data that was not expected, I think the parser was trying to turn :0 into 0 and that was causing tests to fail

jceaser · 2026-09-02T18:45:10Z

-           ["GUEST" "AG10000-PROV"] (concat guest-acls AG10000-acls)
-           ["AG12345-PROV" "AG10000-PROV"] (concat AG12345-acls AG10000-acls)
-           ["guest" "registered" "AG12345-PROV" "AG10000-PROV"] all-acls-no-admin))
+  (u/without-publishing-messages


this was a suggestion to prevent expected exceptions from showing up in the logs

jceaser · 2026-09-02T18:46:55Z

+        (finally
+          ;; always re-enable writes for access control service
+          (u/enable-access-control-writes post-options)
+          (Thread/sleep 500))))


Try/finally was added to ensure that the services was always put back into read/write mode so that other tests could pass. This allows other tests to continue to run and not fail incorrectly. A delay was added to ensure the other service has time to take effect.

This is one of the intermittent errors we would get.

jceaser · 2026-09-02T18:47:01Z

+        (finally
+          ;; Allways re-eneable writes for access control service, even if there was an exception
+          (u/enable-access-control-writes post-options)
+          (Thread/sleep 500))))


Try/finally was added to ensure that the services was always put back into read/write mode so that other tests could pass. This allows other tests to continue to run and not fail incorrectly. A delay was added to ensure the other service has time to take effect.

This is one of the intermittent errors we would get.

jceaser · 2026-09-02T18:47:26Z

@@ -0,0 +1,192 @@
+(ns cmr.system-int-test.access-control.fixtures


nothing to see here, just moving a file into this package

Through GIT compare, it looks like it was added rather than moved

this file was, because git does not have a cp, the file is still needed for the other tests that use it in that package. I was able to do a git mv for the actual test files, but I left the other tests where they were.

jceaser · 2026-09-02T18:47:39Z

+        (has-action-permission? (url/enable-access-control-writes-url) :post admin-read-update-token)
+        (has-action-permission? (url/enable-email-subscription-processing) :post admin-read-update-token)
+        ;; Give the servers time set
+        (Thread/sleep 500)))))


Try/finally was added to ensure that the services was always put back into read/write mode so that other tests could pass. This allows other tests to continue to run and not fail incorrectly. A delay was added to ensure the other service has time to take effect.

This is one of the intermittent errors we would get.

jceaser · 2026-09-02T18:54:36Z

 (use-fixtures :once (fixtures/int-test-fixtures))

-(deftest invalid-search-test
+(deftest ^:serial invalid-search-test


this is the goal of moving the tests, ^:serial only works in the context of the test group it is running in, and moving it here means it is applied to a large scope.

Did you check that this test is actually being run in the build?

as I understand it, tests.group1.edn should run everything in this package and I think I was getting errors before when I had messed up package names and was missing fixtures.

jmaeng72 · 2026-09-02T19:30:32Z

 (use-fixtures :once (fixtures/int-test-fixtures))

-(deftest invalid-search-test
+(deftest ^:serial invalid-search-test


Did you check that this test is actually being run in the build?

jmaeng72 · 2026-09-02T19:33:15Z

@@ -0,0 +1,192 @@
+(ns cmr.system-int-test.access-control.fixtures


Through GIT compare, it looks like it was added rather than moved

jmaeng72 · 2026-09-02T19:34:29Z

-          (group-permission-parameter-subfield-validation params)
-          (group-permission-permission-validation params)))
+  (let [gp (:group-permission params)]
+    (if (and (some? gp) (not (map? gp)))


why would gp ever be null?

eereiter approved these changes Aug 4, 2026

View reviewed changes

jceaser added type: help wanted status: pending and removed type: help wanted labels Aug 10, 2026

jceaser force-pushed the cmr-11368-databine branch from e012631 to 7890190 Compare August 31, 2026 19:57

jceaser requested a review from ibsenc August 31, 2026 22:01

jceaser marked this pull request as ready for review September 1, 2026 14:09

jceaser added 10 commits September 2, 2026 10:11

CMR-11368: Databine, initial test

4c26cc8

CMR-11368: Getting a lot of random fails with this test

9bfbe88

CMR-11368: Getting a lot of random fails with this test - try to mana…

823f9aa

…ge write status better

CMR-11368: applying same fix to another test

5ae90c6

CMR-11368: another test found to be setting read/write, also making t…

b6f58ad

…ests as serial

CMR-11368: another test found to be setting read/write, also making t…

210540b

…ests as serial

CMR-11368: this will fix the map cast issue

e03f656

CMR-11368: adding more sync and tweeking time to lower chance of hitt…

5c70041

…ing the other testing threads

CMR-11368 - moved two test files to system-int-test

88c5f0d

CMR-11368 - clean up from self review

dd40499

jceaser force-pushed the cmr-11368-databine branch from 05d57bc to dd40499 Compare September 2, 2026 14:15

jceaser commented Sep 2, 2026

View reviewed changes

jceaser changed the title ~~CMR-11368: Databine, initial test~~ CMR-11368: Databine test improvements Sep 2, 2026

jceaser requested a review from eereiter September 2, 2026 19:02

jceaser removed the status: pending label Sep 2, 2026

eereiter approved these changes Sep 2, 2026

View reviewed changes

jmaeng72 reviewed Sep 2, 2026

View reviewed changes

		@@ -0,0 +1,192 @@
		(ns cmr.system-int-test.access-control.fixtures

Uh oh!

Conversation

jceaser commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Background

What are the changes?

What areas of the application does this impact?

Required Checklist

Uh oh!

codecov-commenter commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jceaser Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jceaser commented Jul 29, 2026 •

edited

Loading

codecov-commenter commented Jul 29, 2026 •

edited

Loading

jceaser Sep 2, 2026 •

edited

Loading