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 for assigned_article_spec #6063

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,7 @@ en:
active_courses: Active Courses
add_trainings: Please add student trainings to your assignment timeline. Assigning training modules is an essential part of Wiki Ed's best practices.
alerts: Alerts
does_not_exist: The Item doesn't exist. Create a Sandbox page and assign yourself to that to receive suggestions.
Copy link
Member

Choose a reason for hiding this comment

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

What's this for? It doesn't seem to be used directly by the code you've changed. If it's unrelated to fixing the spec, then at minimum it should be done in a separate commit with an explanation in the commit message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for courses with no ratings it throughs an error, of missing.en.does_not_exist because the en.yml file does not contain a defintion for does_not_exist under courses

all_courses: All Courses
already_enrolled: You are already a part of '%{title}'!
already_exists: That already exists for this course!
Expand Down
2 changes: 2 additions & 0 deletions spec/features/assigned_articles_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
visit "/courses/#{course.slug}/articles/assigned"
expect(page).to have_content('Nancy Tuana')
find('a', text: 'Feedback').click
expect(page).to have_no_content(I18n.t('courses.feedback_loading'))
expect(page).to have_selector('textarea.feedback-form')
find('textarea.feedback-form').fill_in with: 'This is a great article!'
Copy link
Member

Choose a reason for hiding this comment

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

find is one of the Capybara methods that does not wait by default, but if the problem you're working around is just that it takes some time for the feedback-form to appear, you probably don't need the wait block, as expect(page).to have_selector does wait by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue i am working on is that the textarea.feedback-form is not rendered imidiatle, because all the users suggestions for that course has to be loaded which takes a bit longer that the normal wait time of Capybara, so the test fails even before the textarea.feedback-form is rendered,

Copy link
Member

Choose a reason for hiding this comment

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

The normal wait time for Capybara is pretty long, and we rarely need to wait longer. I think the issue here is that find('textarea.feedback-form') does not use the wait time, so it will fail immediately if that element has not rendered when it reaches this line. I suspect that the expect you've added above is sufficient, without the .using_wait_time block.

click_button 'Add Suggestion'
find('a', text: 'Delete').click
Expand Down
Loading