signal UI readiness to selenium et al
refs FOO-1869 When Selenium runs, it can occasionally interact with elements on the page that depend on async initializers, like those imported in runOnEveryPageButDontBlockAnythingElse. This causes flakiness in many tests. Canvas now signals its "readiness" state in window.canvasReadyState and emits the event "canvasReadyStateChange" as it transitions from "loading" to "complete". Selenium - and others interested - can observe this property to delay their actions until Canvas is ready. Readiness is currently defined by having run all the async initializers and having loaded all the deferred bundles. test plan: - Revert the fix for g/263638 locally, if it has already been merged. - Run the following command and verify it fails on master > 50% of the time: docker-compose run --rm web \ bundle exec rake canvas:compile_assets_dev && \ docker-compose run --rm web \ bundle exec rspec spec/selenium/users_spec.rb:263 - Checkout this fix and run that command again: it passes 100% of the time. Change-Id: I32bbf480811ecd13f233c7987d98f1a074de6a51 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/263691 Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> Reviewed-by: Aaron Ogata <aogata@instructure.com> QA-Review: Ahmad Amireh <ahmad@instructure.com> Product-Review: Ahmad Amireh <ahmad@instructure.com>
This commit is contained in:
parent
2ffc19c48e
commit
ac07191963
|
@ -22,12 +22,10 @@ require File.expand_path(File.dirname(__FILE__) + '/common')
|
|||
describe "brandableCss JS integration specs" do
|
||||
include_context "in-process server selenium tests"
|
||||
|
||||
EXAMPLE_CDN_HOST = 'https://somecdn.example.com'
|
||||
|
||||
it "sets ENV.asset_host correctly" do
|
||||
expect(Canvas::Cdn.config).to receive(:host).at_least(:once).and_return(EXAMPLE_CDN_HOST)
|
||||
expect(Canvas::Cdn.config).to receive(:host).at_least(:once).and_return(app_url)
|
||||
get "/login/canvas"
|
||||
expect(driver.execute_script("return ENV.ASSET_HOST")).to eq(EXAMPLE_CDN_HOST)
|
||||
expect(driver.execute_script("return ENV.ASSET_HOST")).to eq(app_url)
|
||||
end
|
||||
|
||||
it "loads css from handlebars with variables correctly" do
|
||||
|
|
|
@ -29,8 +29,6 @@ require File.expand_path(File.dirname(__FILE__) + '/common')
|
|||
|
||||
RE_SHORT_MD5 = /\A[a-f0-9]{10}\z/ # 10 chars of an MD5
|
||||
|
||||
EXAMPLE_CDN_HOST = 'https://somecdn.example.com'
|
||||
|
||||
describe 'Stuff related to how we load stuff from CDN and use brandable_css' do
|
||||
include_context "in-process server selenium tests"
|
||||
|
||||
|
@ -75,7 +73,7 @@ describe 'Stuff related to how we load stuff from CDN and use brandable_css' do
|
|||
variant = 'new_styles_normal_contrast'
|
||||
fingerprint = BrandableCSS.cache_for(bundle_name, variant)[:combinedChecksum]
|
||||
expect(fingerprint).to match(RE_SHORT_MD5)
|
||||
url = "#{EXAMPLE_CDN_HOST}/dist/brandable_css/#{variant}/#{bundle_name}-#{fingerprint}.css"
|
||||
url = "#{app_url}/dist/brandable_css/#{variant}/#{bundle_name}-#{fingerprint}.css"
|
||||
assert_tag('link', 'href', url)
|
||||
end
|
||||
|
||||
|
@ -85,12 +83,12 @@ describe 'Stuff related to how we load stuff from CDN and use brandable_css' do
|
|||
expect(asset_path).to be_present
|
||||
end
|
||||
attribute = (tag == 'link') ? 'href' : 'src'
|
||||
url = "#{EXAMPLE_CDN_HOST}#{asset_path}"
|
||||
url = "#{app_url}#{asset_path}"
|
||||
assert_tag(tag, attribute, url)
|
||||
end
|
||||
|
||||
it 'has the right urls for script tag and stylesheets on the login page' do
|
||||
expect(Canvas::Cdn.config).to receive(:host).at_least(:once).and_return(EXAMPLE_CDN_HOST)
|
||||
expect(Canvas::Cdn.config).to receive(:host).at_least(:once).and_return(app_url)
|
||||
get '/login/canvas'
|
||||
|
||||
['bundles/common', 'bundles/login'].each { |bundle| check_css(bundle) }
|
||||
|
|
|
@ -36,6 +36,7 @@ module CustomPageLoaders
|
|||
# force a reload, cuz the `get` above won't
|
||||
driver.navigate.refresh if is_first_request_of_spec
|
||||
close_modal_if_present
|
||||
wait_for_initializers
|
||||
wait_for_ajaximations
|
||||
else
|
||||
wait_for_new_page_load(true) do
|
||||
|
|
|
@ -122,6 +122,7 @@ module CustomValidators
|
|||
driver.switch_to.alert.accept rescue Selenium::WebDriver::Error::NoSuchAlertError
|
||||
driver.execute_script("return window.INST && INST.still_on_old_page !== true;")
|
||||
end or return false
|
||||
wait_for_initializers
|
||||
wait_for_dom_ready
|
||||
wait_for_ajaximations
|
||||
true
|
||||
|
|
|
@ -94,6 +94,23 @@ module CustomWaitMethods
|
|||
wait_for_animations
|
||||
end
|
||||
|
||||
def wait_for_initializers
|
||||
driver.execute_async_script <<~JS
|
||||
var callback = arguments[arguments.length - 1];
|
||||
|
||||
if (window.canvasReadyState === 'complete') {
|
||||
callback()
|
||||
}
|
||||
else {
|
||||
window.addEventListener('canvasReadyStateChange', function() {
|
||||
if (window.canvasReadyState === 'complete') {
|
||||
callback()
|
||||
}
|
||||
})
|
||||
}
|
||||
JS
|
||||
end
|
||||
|
||||
def wait_for_children(selector)
|
||||
has_children = false
|
||||
while has_children == false
|
||||
|
|
33
ui/index.js
33
ui/index.js
|
@ -39,6 +39,31 @@ import './boot/initializers/ajax_errors.js'
|
|||
import './boot/initializers/activateKeyClicks.js'
|
||||
import './boot/initializers/activateTooltips.js'
|
||||
|
||||
window.canvasReadyState = 'loading'
|
||||
window.dispatchEvent(new CustomEvent('canvasReadyStateChange'))
|
||||
|
||||
const readinessTargets = [
|
||||
['asyncInitializers',false],
|
||||
['deferredBundles',false],
|
||||
]
|
||||
const advanceReadiness = target => {
|
||||
const entry = readinessTargets.find(x => x[0] === target)
|
||||
|
||||
if (!entry) {
|
||||
throw new Error(`Invalid readiness target -- "${target}"`)
|
||||
}
|
||||
else if (entry[1]) {
|
||||
throw new Error(`Target already marked ready -- "${target}"`)
|
||||
}
|
||||
|
||||
entry[1] = true
|
||||
|
||||
if (readinessTargets.every(x => x[1])) {
|
||||
window.canvasReadyState = 'complete'
|
||||
window.dispatchEvent(new CustomEvent('canvasReadyStateChange'))
|
||||
}
|
||||
}
|
||||
|
||||
// This is because most pages use this and by having it all in it's own chunk it makes webpack
|
||||
// split out a ton of stuff (like @instructure/ui-view) into multiple chunks because its chunking
|
||||
// algorithm decides that because that chunk would either be too small or it would cause more than
|
||||
|
@ -90,11 +115,15 @@ if (
|
|||
}
|
||||
|
||||
;(window.requestIdleCallback || window.setTimeout)(() => {
|
||||
import('./boot/initializers/runOnEveryPageButDontBlockAnythingElse.js')
|
||||
import('./boot/initializers/runOnEveryPageButDontBlockAnythingElse.js').then(() =>
|
||||
advanceReadiness('asyncInitializers')
|
||||
)
|
||||
})
|
||||
|
||||
ready(() => {
|
||||
;(window.deferredBundles || []).forEach(loadBundle)
|
||||
Promise.all(
|
||||
(window.deferredBundles || []).map(loadBundle)
|
||||
).then(() => advanceReadiness('deferredBundles'))
|
||||
|
||||
// LS-1662: there are math equations on the page that
|
||||
// we don't see, so remain invisible and aren't
|
||||
|
|
Loading…
Reference in New Issue