Skip to content

Fix service tests#7768

Merged
soniaconnolly merged 4 commits intomainfrom
fix-service-tests
Feb 3, 2023
Merged

Fix service tests#7768
soniaconnolly merged 4 commits intomainfrom
fix-service-tests

Conversation

@soniaconnolly
Copy link
Contributor

🛠 Summary of changes

Fix two services tests that failed when the whole services test suite was run locally

  • Fix aes_cipher_spec name dependence on SSL library version
  • Fix classname collision between frontend_logger_spec and analytics_events_ enhancer_spec

soniaconnolly and others added 2 commits February 3, 2023 10:43
Check cipher name by calling library, because names can change.

Co-authored-by: John Maxwell <john.maxwell@gsa.gov>
Co-authored-by: Matt Hinz <matt.hinz@gsa.gov>
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
[skip changelog]

Co-authored-by: John Maxwell <john.maxwell@gsa.gov>
Co-authored-by: Matt Hinz <matt.hinz@gsa.gov>
Comment on lines 10 to 12

class ExampleAnalytics < FakeAnalytics
class ExampleAnalyticsForFL < FakeAnalytics
include ExampleAnalyticsEvents
end
Copy link
Contributor

Choose a reason for hiding this comment

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

ooo I have thoughts about this, I like to create anonymous classes so we don't have to worry about these collisions:

let(:analytics_class) do
  Class.new(FakeAnalytics) do
    include ExampleAnalyticsEvents
  end
end
let(:analytics) { analytics_class.new }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL! Added: 1715e9d

@soniaconnolly soniaconnolly marked this pull request as ready for review February 3, 2023 19:47
@soniaconnolly soniaconnolly requested a review from a team February 3, 2023 19:47
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jmax-gsa jmax-gsa left a comment

Choose a reason for hiding this comment

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

LGTM

The change to using classes created on the fly cleans this up even more; nice.

@soniaconnolly soniaconnolly merged commit 337bacc into main Feb 3, 2023
@soniaconnolly soniaconnolly deleted the fix-service-tests branch February 3, 2023 23:42
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.

3 participants