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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flaky factory when the user's sequence name arrives to 1234 #9167

Merged
merged 1 commit into from Apr 22, 2022

Conversation

andreslucena
Copy link
Member

@andreslucena andreslucena commented Apr 19, 2022

馃帺 What? Why?

On #9164 it was detected a flaky spec that comes from a seed related to #9090.

The culprit is the sequence name, when it arrives to 1234 and the password validation kicks in.

This PR fixes it by removing this sequence in the name.

馃搶 Related Issues

Testing

To test the failing, you can do it with this script:

require "faker"
require "factory_bot"
require "decidim/core/test/factories"

FactoryBot.define do
  factory :user2, class: "Decidim::User" do
    email { "user#{rand(100..10000)}@example.org" } # Changed to skip email already taken validation
    password { "decidim123456" }
    password_confirmation { password }
    name { "#{Faker::Name.name} 1234".delete("'") } # Here's the problem if n arrives to 1234
    nickname { generate(:nickname) }
    organization { Decidim::Organization.first } # Changed to skip organization already taken validation
    tos_agreement { "1" }
  end 
end

FactoryBot.create :user2

鈾ワ笍 Thank you!

Copy link
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

LGTM!

I cannot think of a situation where the name would have to be unique. In real life we can also have people with exactly the same names.

I also checked that generate(:name) is used only for these:

  • User names
  • Dummy resource titles
  • Nested dummy resource titles
  • Coauthorable dummy resource titles
  • In a migratiion spec testing the blocked user name migration

So this seems fine!

@ahukkanen ahukkanen merged commit 50c3a18 into develop Apr 22, 2022
@ahukkanen ahukkanen deleted the test/flaky-user-name-validation-seed branch April 22, 2022 06:41
@andreslucena andreslucena added module: core type: internal PRs that aren't necessary to add to the CHANGELOG for implementers labels Apr 22, 2022
entantoencuanto added a commit to PopulateTools/decidim that referenced this pull request Apr 27, 2022
* chore/load_comments_with_ajax:
  Take into account automatic translations in comments component
  Fix test
  Change comment_cell methods to use descendants recursive query
  Ignore hidden comments when there are not comments at the same level or lower to show
  Add a concern to deal with parent/child recursive resources using pure postgresql queries and use in comments
  Recover hide class removal on replies
  No need to change hide class because now it's loaded automatically from ajax
  Restore changes
  Lint JS
  Don't call on null selector
  Lint offenses
  Don't instantiate counter when there's no textarea
  Fix lint errors
  Add comments loading message
  Load empty comments list in comments cell unless single_comment?
  Send refresh comments request on comments component initialization
  Fix flaky factory when the user's sequence name arrives to 1234 (decidim#9167)
@alecslupu alecslupu added this to the 0.27.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: core type: internal PRs that aren't necessary to add to the CHANGELOG for implementers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants