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

Fix: failing test in account_requests_spec #6065

Conversation

Formasitchijoh
Copy link
Contributor

@Formasitchijoh Formasitchijoh commented Dec 17, 2024

What this PR does

This PR fixes the failing test in account_requests_spec by ensuring that the confirmation modal is visible and the "OK" button is rendered before interacting with it.

Key Changes:

  • Added checks to ensure the modal is visible.
  • Confirmed the "OK" button is available before the test interacts with it.

The initial failure could not be reproduced locally. However, a thorough analysis of the modal's implementation and the associated test suggests that the error is likely caused by the "OK" button not being available at the time the test is executed.

Files Changed:

  • account_request_spec.rb

Screenshots

Before:(Failure on the CI log)
Screenshot 2024-12-17 at 12 13 15
After:
Screenshot 2024-12-17 at 12 12 44

…al is visible and the ok button rendered before interacting with it
@ragesoss
Copy link
Member

Interesting! I assumed this failure was because of the API call to check whether the username was available, which I guessed was failing sometimes because it was happening from GitHub servers affected by a rangeblock.

How confident are you that this fixes the issue? Was it failing before because the 'OK' button was disabled, or not appearing at all? If it wasn't appearing at all, I suspect that this won't fix it, because click_button is one of the Capybara actions that automatically includes waiting behavior.

@Formasitchijoh
Copy link
Contributor Author

Interesting! I assumed this failure was because of the API call to check whether the username was available, which I guessed was failing sometimes because it was happening from GitHub servers affected by a rangeblock.

How confident are you that this fixes the issue? Was it failing before because the 'OK' button was disabled, or not appearing at all? If it wasn't appearing at all, I suspect that this won't fix it, because click_button is one of the Capybara actions that automatically includes waiting behavior.

This particular failure was with respect to requesting to Enable an account, specifically this test was what is failing
it 'can be enabled by the course facilitator' do login_as(instructor) visit "/courses/#{course.slug}" click_button 'Enable account requests' click_button 'OK' expect(page).to have_content('Account request generation enabled') end

It test the Confirm Modal, that has the OK button, this modal is rendered using Javascript, So from the error, the button cannot be found, meaning the modal is not rendered, and the part that says, a button that is not disabled, could be because there are other buttons on the page

@ragesoss
Copy link
Member

That makes sense as a possibility, but I don't know which other (disabled) OK button it would be. I'll merge this, and if it fails again at some point, we can revisit it.

@ragesoss ragesoss merged commit d77c021 into WikiEducationFoundation:master Dec 17, 2024
1 check passed
@Formasitchijoh
Copy link
Contributor Author

Interesting! I assumed this failure was because of the API call to check whether the username was available, which I guessed was failing sometimes because it was happening from GitHub servers affected by a rangeblock.

How confident are you that this fixes the issue? Was it failing before because the 'OK' button was disabled, or not appearing at all? If it wasn't appearing at all, I suspect that this won't fix it, because click_button is one of the Capybara actions that automatically includes waiting behavior.

I’m fairly confident this will address the issue, as the 'OK' button is not disabled. My assumption is that Capybara might have been interacting with the wrong button since the page contains multiple buttons. By explicitly ensuring the modal is visible and rendered before interacting, this fix should reduce ambiguity.

That said, I couldn’t reproduce the failure locally, so I’m addressing this somewhat blindly. If the issue persists, it might require further debugging to identify the exact conditions causing the test to fail.

@Formasitchijoh
Copy link
Contributor Author

That makes sense as a possibility, but I don't know which other (disabled) OK button it would be. I'll merge this, and if it fails again at some point, we can revisit it.

Okay then

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