The Trust and Safety Product Team engineering team recently decided to drop all selenium tests for code that we own because they have been unstable and did not think it was worth the maintenance burden to keep them working, so I think we will likely drop this test and replace it (if possible) with PHPUnit / QUnit tests.

Actually, after discussing with other engineers we have decided to leave this task as the test isn't failing in CI so we don't need to fix it now.

As far as we can see Test Platform will take this work on (if that isn't the case and you are expecting Trust and Safety Product Team to fix the test then we would likely drop the test)

Test Platform has tested several end-to-end testing tools in the past, including Selenium. Our conclusion was that all tools were pretty good. There were no difference in flakiness between tools. The only source of flakiness we were able to find were tests themselves. It's not easy to get end-to-end tests to work robustly. Experience and training are needed. If you need help, let us know.

That said, replacing end-to-end tests with unit and/or integration tests where possible would be my recommendation.

Test Platform can't work on this because we don't really know how the tests should work, or if they actually need Vector or not. We're open to pairing with you on fixing this.

I agree that this is not urgent since the CI is not broken.

That said, tests are passing in CI because it by default installs Vector skin. As far as I know, there's no way to disable that.

Tests will fail with any tool that installs core only, without a skin. We have noticed that while testing MediaWiki-Quickstart (Docker) (local development environment). You can see it's CI output and failing tests at https://quickstart-ci-components.wmcloud.org/

Direct link to the last test run: https://quickstart-ci-components.wmcloud.org/api/results/1757045721.log.ansi.html (It might only be available for some time.)

Change #1187378 had a related patch set uploaded (by Zfilipin; author: Zfilipin):

[mediawiki/core@master] WIP selenium: Delete tests that require Vector skin

https://gerrit.wikimedia.org/r/1187378

Change #1187385 had a related patch set uploaded (by Zfilipin; author: Zfilipin):

[mediawiki/skins/Vector@master] WIP selenium: Move tests from core that require Vector

https://gerrit.wikimedia.org/r/1187385

@Dreamy_Jazz I took a quick look and if I'm not missing anything, both temporary user tests would be fixed if tempUserSignUpButton() changed

from

get tempUserSignUpButton() {
  return $( '.mw-temp-user-banner-buttons > #pt-createaccount' );
}

to

get tempUserSignUpButton() {
  return $( 'a[href="/wiki/Special:CreateAccount"]' );
}

Would that work for you? Am I missing something?

Changing it to that rule means that technically the rule no longer targets specifically the temporary account sign up button. However, I don't think that's a major issue. Therefore, I'd suggest also renaming the function.

Querying by URL seems unstable, specially given that it includes wiki in the URL which may be different wiki-by-wiki.

Would it not be possible to query for pt-createaccount. That ID seems to be added by code that isn't provided by the Vector 2022 skin.

get signUpButton() {
	return $( '#pt-createaccount' );
}

In T404182#11190739, @Dreamy_Jazz wrote:

Querying by URL seems unstable, specially given that it includes wiki in the URL which may be different wiki-by-wiki.

Good point. Something like this could work (not tested.)

get tempUserSignUpButton() {
  return $( 'a[href$="/Special:CreateAccount"]' );
}

Would it not be possible to query for pt-createaccount. That ID seems to be added by code that isn't provided by the Vector 2022 skin.

get signUpButton() {
	return $( '#pt-createaccount' );
}

I don't see #pt-createaccount in the HTML. Am I looking at the wrong place?

In T404182#11192835, @zeljkofilipin wrote:

In T404182#11190739, @Dreamy_Jazz wrote:

Querying by URL seems unstable, specially given that it includes wiki in the URL which may be different wiki-by-wiki.

Good point. Something like this could work (not tested.)

get tempUserSignUpButton() {
  return $( 'a[href$="/Special:CreateAccount"]' );
}

Would it not be possible to query for pt-createaccount. That ID seems to be added by code that isn't provided by the Vector 2022 skin.

get signUpButton() {
	return $( '#pt-createaccount' );
}

I don't see #pt-createaccount in the HTML. Am I looking at the wrong place?

I had assumed that #pt-createaccount would exist somewhere, because it is an ID for an element defined in MediaWiki core. I guess it's instead included by Vector 2022 and therefore not displayed not by default.

P83422 Special:CreateAccount

1	<div role="status" class="mw-notification mw-notification-autohide postedit-tempuserpopup mw-notification-visible">
2	<div class="mw-notification-title">Temporary account</div>
3	<div class="mw-notification-content">
4	An
5	<a title="Mw:Special:MyLanguage/Help:Temporary accounts" href="/wiki/Mw:Special:MyLanguage/Help:Temporary_accounts">
6	auto-generated account
7	</a>
8	has been created for you by adding a cookie to your browser. To pick a name of your choice and get more features, consider
9	<a href="/wiki/Special:CreateAccount">
10	creating an account
11	</a>
12	.
13	</div>
14	</div>

That notification only appears once when the temporary account is created and does disappear after a time. This time period is 30 seconds (defined here as 'long' which means 30 seconds). If the tests are running slowly it may be possible for the notification to disappear and then the test would fail.

As such, I would recommend instead simply opening the create account page once the edit to create the temporary account has completed. So something like a super.openTitle call for the create account special page.

In T404182#11192953, @Dreamy_Jazz wrote:

As such, I would recommend instead simply opening the create account page once the edit to create the temporary account has completed. So something like a super.openTitle call for the create account special page.

I'm lost. :) Could you push a patch to Gerrit so I could test this?

In T404182#11166851, @Dreamy_Jazz wrote:

As far as we can see Test Platform will take this work on (if that isn't the case and you are expecting Trust and Safety Product Team to fix the test then we would likely drop the test)

I'm not sure if I made it explicit, but I know how to make the tests pass, but I don't know if the fix is doing the right thing. I didn't understand your comments on what I should do.

From what I saw so far, the fix is almost trivial (one line of code, a few lines at the most), but I can't do it alone, I don't have any context on how temporary accounts should work.

1. Would you prefer to delete the tests? I can work on finishing 1187378.
2. Would you prefer to pair on this next week so we can fix the tests together? My estimate is that it would take us 30-60 minutes.

Change #1187385 abandoned by Zfilipin:

[mediawiki/skins/Vector@master] WIP selenium: Move tests from core that require Vector

Reason:

The fix to make the tests work seems to be trivial.

https://gerrit.wikimedia.org/r/1187385

@zeljkofilipin our team doesn't have time to work on this issue. From our point of view, the tests work in CI, and do the job that they need to do--specifically, validating via an end-to-end test that editing results in a temporary account being created, and this has a dependency on the Vector skin, in that temporary accounts have full UI support in Vector and Minerva. If your team would like to move the test to a different location or rewrite the test such that it passes without Vector or Minerva being enabled, we don't have capacity to support that.

Change #1187378 abandoned by Zfilipin:

[mediawiki/core@master] WIP selenium: Delete tests that require Vector skin

https://gerrit.wikimedia.org/r/1187378

Change #1211136 had a related patch set uploaded (by Zfilipin; author: Zfilipin):

[mediawiki/core@master] WIP selenium: Temporary user tests work without Vector skin

https://gerrit.wikimedia.org/r/1211136

Change #1211674 had a related patch set uploaded (by Zfilipin; author: Zfilipin):

[mediawiki/core@master] WIP selenium: debug tempUserSignUpButton()

https://gerrit.wikimedia.org/r/1211674

Change #1211674 abandoned by Zfilipin:

[mediawiki/core@master] WIP selenium: debug tempUserSignUpButton()

https://gerrit.wikimedia.org/r/1211674

Change #1187385 restored by Zfilipin:

[mediawiki/skins/Vector@master] WIP selenium: Move tests from core that require Vector

https://gerrit.wikimedia.org/r/1187385

Change #1187378 restored by Zfilipin:

[mediawiki/core@master] WIP selenium: Delete tests that require Vector skin

https://gerrit.wikimedia.org/r/1187378

Change #1211136 abandoned by Zfilipin:

[mediawiki/core@master] WIP selenium: Temporary user tests work without Vector skin

https://gerrit.wikimedia.org/r/1211136

Change #1211136 restored by Dreamy Jazz:

[mediawiki/core@master] WIP selenium: Temporary user tests work without Vector skin

https://gerrit.wikimedia.org/r/1211136

All Selenium core tests pass with just core installed. No skin is required for core tests to pass.

this seems strange to me. All our users use a skin to access our projects so skins should be required. If we are not running those tests with a skin they lose their value as they are not testing end to end what we want to test! All tests require a skin to work.

These tests should not be added to Vector repo. These tests are valuable and support products built by Product Safety and Integrity. The Vector and Minerva skins are owned by the Reader Experience teams and the tests there support those teams. Having tests owned by one team in a repo that is not owned by that team doesn't make sense.

In general, I think we might want to step back and reconsider how we ship and run tests. I think we might benefit from moving all our Selenium tests to a dedicated repo with an OWNERS.md file documenting who owns which test. I think it would have lots of advantages. If it's useful in January/Feb perhaps some principal/senior engineers in WMF product could talk to Test-Platform about this.

Change #1211136 merged by jenkins-bot:

[mediawiki/core@master] selenium: Update edit.page.js to not rely on Vector being installed

https://gerrit.wikimedia.org/r/1211136

Change #1187378 abandoned by Dreamy Jazz:

[mediawiki/core@master] selenium: Delete tests that require Vector skin

Reason:

I009eed53e8d513334b98a1f859a79b58d4532de0 has been merged

https://gerrit.wikimedia.org/r/1187378

Change #1187385 abandoned by Dreamy Jazz:

[mediawiki/skins/Vector@master] selenium: Move temporary user tests from core

Reason:

I009eed53e8d513334b98a1f859a79b58d4532de0 has been merged

https://gerrit.wikimedia.org/r/1187385

@zeljkofilipin can you test that the tests now pass using your quickstart environment?

@Dreamy_Jazz MediaWiki-Quickstart (Docker) has CI for core and all supported extensions and skins. (CI runs in a loop, it needs a few hours for every loop.)

https://quickstart-ci-components.wmcloud.org/

As soon as the CI runs again, ./run_selenium_tests at the top of the page should change from red x to green check-mark.

@Dreamy_Jazz apologies for being slow on this. Yes, temporary user tests are now passing without Vector skin installed. Thanks for the patch!