-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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: Launchpad perpetual loading state, in certain circumstances #29597
fix: Launchpad perpetual loading state, in certain circumstances #29597
Conversation
* conditions for refetch are: | ||
* - There is a current project, but Config file has not yet loaded | ||
* - There are no pending (delayed) refetches, or fetches in progress | ||
* - There is no baseError - we don't want to continue to refetch if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to have a test for the error scenario?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in c15a55d - thanks, it found a bug!
Passing run #55632 ↗︎
Details:
Review all test suite changes for PR #29597 ↗︎ |
Co-authored-by: Ryan Manuel <[email protected]>
Co-authored-by: Bill Glesias <[email protected]>
Co-authored-by: Bill Glesias <[email protected]>
Co-authored-by: Bill Glesias <[email protected]>
Co-authored-by: Bill Glesias <[email protected]>
Released in This comment thread has been locked. If you are still experiencing this issue after upgrading to |
Additional details
This bug was evidenced by the flaky tests in the linked issue, but may have other manifestations that will clear up with this PR.
The
currentProject
schema on theMainLaunchpadQuery
GraphQL query, used by Launchpad'sMain.vue
, has a field namedisLoadingConfigFile
. This is a boolean field, and both true and false fields are valid for it.Previous to this PR, if this field was true for the first
MainLaunchpadQuery
request, the main launchpad UI would get stuck in a loading state: the rest of the launchpad cannot operate properly if the config file has not yet loaded, so we gate a lot of functionality behind that field.In the linked flaky tests, we execute two custom tasks:
scaffoldProject
andopenProject
.scaffoldProject
initializes one of the projects fromsystem-tests
, and installs dependencies for it.openProject
spawns the Cypress process that will execute against the scaffolded project, and initializes some data contexts. That kicks off the process that loads the config file, but the task does not wait for that loading to be complete before resolving. Sometimes the config file loads quickly - before the outer e2e runner can load the launchpad under test - and the flaky tests pass. Other times the config file loads during or just after theMainLaunchpadQuery
request, which causes the flaky tests to fail - the inner loading spinner inMain.vue
is never removed, becauseisLoadingConfigFile
is false on first render of the query's data.This is labeled as a
fix
rather thanflake
because this path, from what I can tell, is possible for end users to experience, though it is likely a rare occurrence.The solution implemented is simply to poll (with a geometrically increasing delay) the
MainLaunchpadQuery
until the config file loads. This does not have a max count, because thebaseError
of this query is set up as a subscription - if the config file fails to load, the error gets propagated and the spinner is replaced with an error message.Another option to consider would be to make config (and related fields in the
@packages/graphql
schema) a first-class type and resolver, where the root resolver wouldawait
the process of loading the config file. This was determined to be a fairly large change, however, when a simple poll would fix the manifestation of the bug.Steps to test
Execute the
config loading state
block in@packages/launchpad
'sproject-setup.cy.ts
spec. The first request to theMainLaunchpadQuery
resolves withisLoadingConfigFile: true
, while the second request resolves withisLoadingConfigFile: false
. Any subsequent calls will throw an error.How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?