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

Refactor system tests to pass in DevContainers #5271

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
4 changes: 1 addition & 3 deletions .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,4 @@
ARG RUBY_VERSION=3.3.5
FROM ghcr.io/rails/devcontainer/images/ruby:$RUBY_VERSION

RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \
&& apt-get -y install --no-install-recommends pkg-config \
&& apt-get clean && rm -rf /var/lib/apt/lists/*
RUN sudo apt-get update && sudo apt-get -y install --no-install-recommends pkg-config
2 changes: 2 additions & 0 deletions .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
"containerEnv": {
"EDITOR": "code --wait",
"GIT_EDITOR": "code --wait",
"DEVCONTAINER_APP_HOST": "http://rails-app",
"MEMCACHED_ENDPOINT": "cache:11211",
"CAPYBARA_SERVER_PORT": "45678",
"SELENIUM_HOST": "selenium",
"ELASTICSEARCH_URL": "http://search:9200",
Expand Down
2 changes: 1 addition & 1 deletion app/views/layouts/_session.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<div class="ml-3 flex text-neutral-900 dark:text-white hover:text-black dark:hover:text-neutral-400 items-center" data-controller="dialog">
<% if signed_in? %>
<%# This class is used in the tests :( I need it to be the same class until the old design is gone. %>
<%= link_to(dashboard_path, data: { action: "dialog#open", dialog_target: "button" }, class: "header__popup-link") do %>
<%= link_to(dashboard_path, data: { action: "dialog#open", dialog_target: "button" }, class: "header__popup-link", title: "User Dropdown Menu") do %>
<%= avatar 64, "user_gravatar", theme: :dark, class: "h-9 w-9 rounded" %>
<% end %>
<% else %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<span><%= current_user.name %></span>
<%= avatar 80, "user_gravatar", theme: :dark %>
</a>
<a href="#" class="header__popup-link" data-icon="▼" data-action="dropdown#toggle click@window->dropdown#hide">
<a href="#" title="User Dropdown Menu" class="header__popup-link" data-icon="▼" data-action="dropdown#toggle click@window->dropdown#hide">
<span class="t-hidden">More items</span>
</a>
<div class="header__popup__nav-links hidden" data-dropdown-target="menu" >
Expand Down
2 changes: 1 addition & 1 deletion config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
# Show full error reports and disable caching.
config.consider_all_requests_local = true
config.action_controller.perform_caching = false
config.cache_store = :mem_cache_store
config.cache_store = :mem_cache_store, ENV["MEMCACHED_ENDPOINT"]

# Raise exceptions instead of rendering exception templates.
config.action_dispatch.show_exceptions = :none
Expand Down
6 changes: 5 additions & 1 deletion config/initializers/webauthn.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@
config.origin = if Rails.env.development?
ENV.fetch("WEBAUTHN_ORIGIN", "http://localhost:3000")
elsif Rails.env.test?
"#{Gemcutter::PROTOCOL}://#{Gemcutter::HOST}:31337"
if ENV["DEVCONTAINER_APP_HOST"].present?
"#{ENV['DEVCONTAINER_APP_HOST']}:#{ENV['CAPYBARA_SERVER_PORT']}"
else
"#{Gemcutter::PROTOCOL}://#{Gemcutter::HOST}:31337"
end
else
"#{Gemcutter::PROTOCOL}://#{Gemcutter::HOST}"
end
Expand Down
8 changes: 7 additions & 1 deletion test/application_system_test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,14 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
driven_by :selenium, using: :headless_chrome, screen_size: [1400, 1400], options: {
browser: :remote,
url: "http://#{ENV['SELENIUM_HOST']}:4444"
}
} do |driver|
driver.add_argument("--unsafely-treat-insecure-origin-as-secure=http://rails-app:#{ENV['CAPYBARA_SERVER_PORT']}")
end
else
driven_by :selenium, using: :headless_chrome, screen_size: [1400, 1400]
end

def devcontainer?
ENV["DEVCONTAINER_APP_HOST"].present?
end
end
2 changes: 1 addition & 1 deletion test/helpers/email_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ def confirmation_link
refute_empty ActionMailer::Base.deliveries
body = last_email.parts[1].body.decoded.to_s
link = %r{http://localhost(?::\d+)?/email_confirmations([^";]*)}.match(body)
link[0]
URI.parse(link[0]).request_uri
end
end
20 changes: 10 additions & 10 deletions test/system/api_keys_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class ApiKeysTest < ApplicationSystemTestCase

assert_nil URI.parse(page.current_url).query

fill_in "api_key[name]", with: "test"
fill_in "api_key[name]", with: "test-api-key"
simi marked this conversation as resolved.
Show resolved Hide resolved
check "api_key[index_rubygems]"

refute page.has_content? "Enable MFA"
Expand All @@ -28,7 +28,7 @@ class ApiKeysTest < ApplicationSystemTestCase
assert_nil @user.api_keys.last.rubygem

assert_event Events::UserEvent::API_KEY_CREATED, {
name: "test",
name: "test-api-key",
scopes: ["index_rubygems"],
mfa: false,
api_key_gid: @user.api_keys.last.to_global_id.to_s
Expand All @@ -43,7 +43,7 @@ class ApiKeysTest < ApplicationSystemTestCase

assert_empty URI.parse(page.current_url).query

fill_in "api_key[name]", with: "test"
fill_in "api_key[name]", with: "test-api-key"
check "api_key[index_rubygems]"

refute page.has_content? "Enable MFA"
Expand All @@ -58,7 +58,7 @@ class ApiKeysTest < ApplicationSystemTestCase
test "creating new api key scoped to a gem" do
visit_profile_api_keys_path

fill_in "api_key[name]", with: "test"
fill_in "api_key[name]", with: "test-api-key"
check "api_key[push_rubygem]"

assert page.has_select? "api_key_rubygem_id", selected: "All Gems"
Expand All @@ -82,7 +82,7 @@ class ApiKeysTest < ApplicationSystemTestCase
ApiKey::APPLICABLE_GEM_API_SCOPES.each do |scope|
test "creating new api key scoped to a gem with #{scope} scope" do
visit_profile_api_keys_path
fill_in "api_key[name]", with: "test"
fill_in "api_key[name]", with: "test-api-key"
check "api_key[#{scope}]"

assert page.has_select? "api_key_rubygem_id", selected: "All Gems"
Expand All @@ -96,7 +96,7 @@ class ApiKeysTest < ApplicationSystemTestCase

test "selecting the exclusive scope deselects the other scopes and vice versa" do
visit_profile_api_keys_path
fill_in "api_key[name]", with: "test"
fill_in "api_key[name]", with: "test-api-key"
check "api_key[index_rubygems]"
check "api_key[push_rubygem]"

Expand Down Expand Up @@ -125,7 +125,7 @@ class ApiKeysTest < ApplicationSystemTestCase
test "creating new api key scoped to gem that the user does not own" do
visit_profile_api_keys_path

fill_in "api_key[name]", with: "test"
fill_in "api_key[name]", with: "test-api-key"
check "api_key[push_rubygem]"

assert page.has_select? "api_key_rubygem_id", selected: "All Gems"
Expand All @@ -144,7 +144,7 @@ class ApiKeysTest < ApplicationSystemTestCase

visit_profile_api_keys_path

fill_in "api_key[name]", with: "test"
fill_in "api_key[name]", with: "test-api-key"
check "api_key[index_rubygems]"
check "mfa"
click_button "Create API Key"
Expand All @@ -158,7 +158,7 @@ class ApiKeysTest < ApplicationSystemTestCase

expiration = 1.day.from_now.beginning_of_minute

fill_in "api_key[name]", with: "test"
fill_in "api_key[name]", with: "test-api-key"
check "api_key[index_rubygems]"
fill_in "api_key[expires_at]", with: expiration
click_button "Create API Key"
Expand All @@ -173,7 +173,7 @@ class ApiKeysTest < ApplicationSystemTestCase

visit_profile_api_keys_path

fill_in "api_key[name]", with: "test"
fill_in "api_key[name]", with: "test-api-key"
check "api_key[index_rubygems]"
click_button "Create API Key"

Expand Down
6 changes: 3 additions & 3 deletions test/system/avo/manual_changes_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class Avo::ManualChangesSystemTest < ApplicationSystemTestCase

page.find_by_id("log_ticket_key", wait: Capybara.default_max_wait_time) # Wait for input to be available.
fill_in "Key", with: "key"
fill_in "Directory", with: "dir"
fill_in "Directory", with: "directory"
fill_in "Processed count", with: "0"
fill_in "Comment", with: "A nice long comment"
click_on "Save"
Expand Down Expand Up @@ -45,7 +45,7 @@ class Avo::ManualChangesSystemTest < ApplicationSystemTestCase
},
"fields" => {
"key" => "key",
"directory" => "dir",
"directory" => "directory",
"backend" => "s3",
"status" => "pending",
"processed_count" => "0"
Expand Down Expand Up @@ -95,7 +95,7 @@ class Avo::ManualChangesSystemTest < ApplicationSystemTestCase
},
"fields" => {
"key" => "Other Key",
"directory" => "dir",
"directory" => "directory",
"backend" => "s3",
"status" => "failed",
"processed_count" => "2"
Expand Down
6 changes: 6 additions & 0 deletions test/system/avo/rubygems_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ class Avo::RubygemsSystemTest < ApplicationSystemTestCase

include ActiveJob::TestHelper

setup do
# Avo records the IP Address and other IP related information when not using `localhost`, which is
# not something easily to calculate.
skip "This test cannot run in the current environment" if devcontainer?
end

test "release reserved namespace" do
admin_user = create(:admin_github_user, :is_admin)
avo_sign_in_as admin_user
Expand Down
6 changes: 6 additions & 0 deletions test/system/avo/users_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ class Avo::UsersSystemTest < ApplicationSystemTestCase

include ActiveJob::TestHelper

setup do
# Avo records the IP Address and other IP related information when not using `localhost`, which is
# not something easily to calculate.
skip "This test cannot run in the current environment" if devcontainer?
end

test "reset mfa" do
Minitest::Test.make_my_diffs_pretty!
admin_user = create(:admin_github_user, :is_admin)
Expand Down
6 changes: 6 additions & 0 deletions test/system/avo/versions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ class Avo::VersionsSystemTest < ApplicationSystemTestCase

include ActiveJob::TestHelper

setup do
# Avo records the IP Address and other IP related information when not using `localhost`, which is
# not something easily to calculate.
skip "This test cannot run in the current environment" if devcontainer?
end

test "restore a rubygem version" do
admin_user = create(:admin_github_user, :is_admin)
avo_sign_in_as admin_user
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require "test_helper"
require "application_system_test_case"
require "helpers/email_helpers"

class EmailConfirmationTest < SystemTest
class EmailConfirmationTest < ApplicationSystemTestCase
include ActiveJob::TestHelper

setup do
Expand All @@ -19,7 +19,7 @@ def request_confirmation_mail(email)
test "requesting confirmation mail does not tell if a user exists" do
request_confirmation_mail "[email protected]"

assert page.has_content? "We will email you confirmation link to activate your account if one exists."
assert_text "We will email you confirmation link to activate your account if one exists."
end

test "requesting confirmation mail with email of existing user" do
Expand All @@ -30,8 +30,8 @@ def request_confirmation_mail(email)
assert_not_nil link
visit link

assert page.has_content? "Sign in"
assert page.has_selector? "#flash_notice", text: "Your email address has been verified"
assert_text "Sign in"
assert_selector "#flash_notice", text: "Your email address has been verified"
end

test "re-using confirmation link, asks user to double check the link" do
Expand All @@ -40,13 +40,13 @@ def request_confirmation_mail(email)
link = last_email_link
visit link

assert page.has_content? "Sign in"
assert page.has_selector? "#flash_notice", text: "Your email address has been verified"
assert_text "SIGN IN"
assert_selector "#flash_notice", text: "Your email address has been verified"

visit link

assert page.has_content? "Sign in"
assert page.has_selector? "#flash_alert", text: "Please double check the URL or try submitting it again."
assert_text "SIGN IN"
assert_selector "#flash_alert", text: "Please double check the URL or try submitting it again."
end

test "requesting multiple confirmation email" do
Expand Down Expand Up @@ -77,8 +77,8 @@ def request_confirmation_mail(email)
fill_in "otp", with: ROTP::TOTP.new(@user.totp_seed).now
click_button "Authenticate"

assert page.has_content? "Sign in"
assert page.has_selector? "#flash_notice", text: "Your email address has been verified"
assert_text "Sign in"
assert_selector "#flash_notice", text: "Your email address has been verified"
end

test "requesting confirmation mail with webauthn enabled" do
Expand All @@ -91,15 +91,15 @@ def request_confirmation_mail(email)
assert_not_nil link
visit link

assert page.has_content? "Multi-factor authentication"
assert page.has_content? "Security Device"
assert_text "Multi-factor authentication"
assert_text "Security Device"

click_on "Authenticate with security device"

assert page.has_content? "Sign in"
assert_text "SIGN IN"
skip("There's a glitch where the webauthn javascript(?) triggers the next page to render twice, clearing flash.")

assert page.has_selector? "#flash_notice", text: "Your email address has been verified"
assert_selector "#flash_notice", text: "Your email address has been verified"
end

test "requesting confirmation mail with webauthn enabled using recovery codes" do
Expand All @@ -112,14 +112,14 @@ def request_confirmation_mail(email)
assert_not_nil link
visit link

assert page.has_content? "Multi-factor authentication"
assert page.has_content? "Security Device"
assert_text "Multi-factor authentication"
assert_text "Security Device"

fill_in "otp", with: @mfa_recovery_codes.first
click_button "Authenticate"

assert page.has_content? "Sign in"
assert page.has_selector? "#flash_notice", text: "Your email address has been verified"
assert_text "Sign in"
assert_selector "#flash_notice", text: "Your email address has been verified"
end

test "requesting confirmation mail with mfa enabled, but mfa session is expired" do
Expand All @@ -135,7 +135,7 @@ def request_confirmation_mail(email)
travel 16.minutes do
click_button "Authenticate"

assert page.has_content? "Your login page session has expired."
assert_text "Your login page session has expired."
end
end

Expand Down
2 changes: 0 additions & 2 deletions test/system/multifactor_auths_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,6 @@ class MultifactorAuthsTest < ApplicationSystemTestCase
end

should "user with webauthn can change mfa level" do
fullscreen_headless_chrome_driver

sign_in
visit edit_settings_path

Expand Down
Loading
Loading