Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RED-2215] Fix/Improve Live Specs #573

Merged
merged 1 commit into from
Jun 14, 2024
Merged

[RED-2215] Fix/Improve Live Specs #573

merged 1 commit into from
Jun 14, 2024

Conversation

fbvilela
Copy link
Contributor

@fbvilela fbvilela commented Jun 4, 2024

Description

Fix and Improve the Live Specs

@fbvilela fbvilela force-pushed the fvilela/spec_tests branch 4 times, most recently from 0a13e99 to 8373b02 Compare June 13, 2024 05:23
@fbvilela fbvilela changed the title See live tests, ignore [RED-2215] Fix/Improve Live Specs Jun 13, 2024
@fbvilela fbvilela force-pushed the fvilela/spec_tests branch 2 times, most recently from e21dbec to c4617a9 Compare June 13, 2024 05:51
@fbvilela fbvilela marked this pull request as ready for review June 13, 2024 05:52
@fbvilela fbvilela requested a review from a team as a code owner June 13, 2024 05:52
Copy link
Contributor

@ecoologic ecoologic left a comment

Choose a reason for hiding this comment

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

✅ - good to merge, but I had a few neat picks I couldn't resist. Just if you have time, no biggie.

c.allow_http_connections_when_no_cassette = @previous_allow_http_connections
end
end

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm OK with this solution. 👍

@@ -21,26 +21,26 @@ def it_should_be_creatable(options = {})

before(:all) do
VCR.use_cassette("#{described_class.to_s}_create") do
@object = described_class.create(client, valid_attributes.merge(default_options))
@creatable_object = described_class.create!(client, valid_attributes.merge(default_options))
Copy link
Contributor

Choose a reason for hiding this comment

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

I love the bang! ❤️

@object.send("#{attribute}=", value)
extra.each { |k, v| @object.send("#{k}=", v) }
@updatable_object.send("#{attribute}=", value)
extra.each { |k, v| @updatable_object.send("#{k}=", v) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Neat: would it be possible to use public_send instead?

end

it "should be savable" do
expect(@object.save).to be(true)
expect(@updatable_object.save!).to be(true)
Copy link
Contributor

@ecoologic ecoologic Jun 13, 2024

Choose a reason for hiding this comment

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

In this case I think it should be save without the bang.

You could try:

Suggested change
expect(@updatable_object.save!).to be(true)
expect(object.save).to be(true), "Expected object to save, but it failed with errors: #{object.errors.full_messages.join(', ')}"

@fbvilela fbvilela merged commit 48e7438 into master Jun 14, 2024
7 checks passed
@fbvilela fbvilela deleted the fvilela/spec_tests branch June 14, 2024 04:49
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