Skip to content

changelog: Internal, Tests, Updating some tests to be easier to read#11352

Merged
Sgtpluck merged 1 commit intomainfrom
dmm/update-attribute-asserter-spec
Oct 17, 2024
Merged

changelog: Internal, Tests, Updating some tests to be easier to read#11352
Sgtpluck merged 1 commit intomainfrom
dmm/update-attribute-asserter-spec

Conversation

@Sgtpluck
Copy link
Contributor

🛠 Summary of changes

I was working on a change that required some tests added to this spec file, and I got fed up with how it is structured, so I updated it to use a factory object rather than a double. This allowed me to hydrate the service provider with the correct data, rather than having to rely on stubs. The change ended up being out of scope of the other change, so I pulled it into its own branch for ease of review.

Copy link
Contributor

@lmgeorge lmgeorge left a comment

Choose a reason for hiding this comment

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

LGTM! Really like how this makes the focus on what attributes are shown more clear.

Comment on lines -88 to -93
before do
user.identities << identity
allow(service_provider.metadata).to receive(:[]).with(:attribute_bundle).
and_return(%w[email phone first_name])
subject.build
end
Copy link
Contributor

Choose a reason for hiding this comment

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

🥇 for getting rid of all this boilerplate setup

@Sgtpluck Sgtpluck merged commit 8ff1501 into main Oct 17, 2024
@Sgtpluck Sgtpluck deleted the dmm/update-attribute-asserter-spec branch October 17, 2024 12:40
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.

2 participants