Skip to content

Make unit tests pass with ROX_POSTGRES_DATASTORE flag both on and off#4719

Merged
connorgorman merged 7 commits intomasterfrom
cgorman-test-flag-on
Feb 6, 2023
Merged

Make unit tests pass with ROX_POSTGRES_DATASTORE flag both on and off#4719
connorgorman merged 7 commits intomasterfrom
cgorman-test-flag-on

Conversation

@connorgorman
Copy link
Copy Markdown
Contributor

@connorgorman connorgorman commented Feb 5, 2023

Description

Split out the code changes that would not need to be reverted based on the flag flip from #4690

These tests need to be ported over to using Postgres as they test functionality not dependent on RocksDB

Central/deployment/index/search_comparison_test.go
Central/splunk/violations_test
TestImageLoader - graphql/resolvers/loaders/images_test.go
TestImageIntegrationDataStore - central/imageintegration/datastore/datastore_impl.go
TestProcessBaselineService - central/processbaseline/service/service_impl_test.go
Role/datastore/datastore_test.go

Once merged, will create JIRAs

Checklist

  • Investigated and inspected CI test results
  • Unit test and regression tests added
  • Evaluated and added CHANGELOG entry if required
  • Determined and documented upgrade steps
  • Documented user facing changes (create PR based on openshift/openshift-docs and merge into rhacs-docs)

If any of these don't apply, please comment below.

Testing Performed

Unit tests :)

@ghost
Copy link
Copy Markdown

ghost commented Feb 5, 2023

Images are ready for the commit at f39ec8c.

To use with deploy scripts, first export MAIN_IMAGE_TAG=3.74.x-14-gf39ec8c004.

t.Skip("Skip postgres tests")
t.SkipNow()
}
pgtest.SkipIfPostgresEnabled(t)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this one previously being skipped in Rocks mode?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was running

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one needs to run in Postgres and not run in Rocks. Your change turns it off for Postgres. This is with the updated node CVE structure which requires Postgres unless i'm mistaken.


func (s *TestReportConfigurationServiceTestSuite) SetupTest() {
s.mockCtrl = gomock.NewController(s.T())
if env.PostgresDatastoreEnabled.BooleanSetting() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the new pgtest function?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort of trying to minimize changes tbh. This will eventually be all removed when pg flag is removed

Copy link
Copy Markdown
Contributor

@dashrews78 dashrews78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@connorgorman connorgorman enabled auto-merge (squash) February 6, 2023 18:30
@connorgorman connorgorman disabled auto-merge February 6, 2023 18:33
@connorgorman connorgorman enabled auto-merge (squash) February 6, 2023 18:39
@connorgorman connorgorman disabled auto-merge February 6, 2023 18:40
@connorgorman connorgorman enabled auto-merge (squash) February 6, 2023 19:50
@connorgorman connorgorman disabled auto-merge February 6, 2023 19:50
@connorgorman connorgorman changed the title Make unit tests pass with both flag on and flag off Make unit tests pass with ROX_POSTGRES_DATASTORE flag both on and off Feb 6, 2023
@connorgorman connorgorman enabled auto-merge (squash) February 6, 2023 19:50
@connorgorman connorgorman disabled auto-merge February 6, 2023 21:47
@connorgorman connorgorman enabled auto-merge (squash) February 6, 2023 22:14
@connorgorman connorgorman merged commit 720cd6f into master Feb 6, 2023
@connorgorman connorgorman deleted the cgorman-test-flag-on branch February 6, 2023 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants