We are planning to make the super sidebar navigation the default navigation. Unfortunately we currently rely on the old navigation behavior in more than 150 spec files. Currently the old navigation is used in all specs. So far we have done:
Introduce a user specific trait and move from a global "old"-nav switch to a per-test switch:
Pick one or (more related specs from the sheet), add your name to the Who? column
Similar to button migrations, use some common sense when it comes to picking up multiple specs in one MR.
Side-note: spec/features only needs QA review!
Remove :no_super_sidebar trait from the specs
Fix them
Mark them as done once the MR is merged
I think the specs fall into these categories:
Easy fix by fixing some css selectors
Spec can be removed because they tested an old-nav behavior that isn't here anymore
Controller specs: There are a few controller specs that guard against N+1 queries. Ignore them for now. I believe they will be fixed once we remove the toggle.
If you are unsure about the correct group, please do not leave the issue without a group label, and refer to
GitLab's shared responsibility functionality guidelines
for more information on how to triage this kind of issue.
This issue was automatically tagged with the label groupfoundations by TanukiStan, a machine learning classification model, with a probability of 0.73.
If this label is incorrect, please tag this issue with the correct group label as well as automation:ml wrong to help TanukiStan learn from its mistakes.
Hey @markrian@pgascouvaillancourt@sdejonge@thutterer, assigning y'all to this issue for the next milestone! Hopefully the description is quite clear. Please let me know, if you have any questions.
I will go and pick up #420119 (closed) first, and then also help burn down these!
Lukas Eipertchanged title from Super sidebar logged-in specs: Move from global feature flag disabled to per-spec to Super sidebar logged-in specs: Change specs to work with new sidebar
changed title from Super sidebar logged-in specs: Move from global feature flag disabled to per-spec to Super sidebar logged-in specs: Change specs to work with new sidebar
{nav_item: _('Project information'),# Category, but also thinks like `Project`# These are all sub menu items, which would be visible in the flyout menunav_sub_items: [_('Activity'),_('Labels'),_('Members')]}
So these specs are kind of snapshot specs with a certain serializer. The question is:
Do we see a benefit in this methodology of directly testing the DOM in feature specs. Should we port the specs to work with the super sidebar?
Should we migrate the tests and just test the JSON which we provide to the super sidebar?
Alternative ???
About forty of the specs that use the trait are of those kind:
I've had to add :js to a few tests to ensure they render the super sidebar for the gitlab_sign_out method in !134840 (merged). The MR has a hightlighted a performance hit for using :js!134840 (comment 1614251949), from docs:
:js is particularly important to avoid. This must only be used if the feature test requires JavaScript reactivity in the browser (for example, clicking a Vue.js component). Using a headless browser is much slower than parsing the HTML response from the app.
I've tried to ensure :js is only on the tests that require it, is there another way to minimise performance impact from these changes?
I'm not aware of any other way, no In any case, I'm sceptical that the reported slow tests are slow due to the changes in the MR. All else being equal, I think there's too much noise in job runtime to determine slow tests from a single pipeline run.