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

[CI] Cypressがfailになる #13605

Closed
1 task
samunohito opened this issue Mar 20, 2024 · 17 comments · Fixed by #13709
Closed
1 task

[CI] Cypressがfailになる #13605

samunohito opened this issue Mar 20, 2024 · 17 comments · Fixed by #13709
Labels
⚠️bug? This might be a bug 🛠️Dev Development of Misskey itself

Comments

@samunohito
Copy link
Member

💡 Summary

Cypressのテストが正常に動作しなくなっている模様。
CypressのUIを起動した状態で動きを見てみると、最初のLoading画面でフリーズしたままタイムアウトしている。

🥰 Expected Behavior

passする

🤬 Actual Behavior

failになる

📝 Steps to Reproduce

cypressで作られた任意のテストを起動

💻 Frontend Environment

-

🛰 Backend Environment (for server admin)

-

Do you want to address this bug yourself?

  • Yes, I will patch the bug myself and send a pull request
@samunohito samunohito added the ⚠️bug? This might be a bug label Mar 20, 2024
@samunohito samunohito mentioned this issue Mar 20, 2024
5 tasks
@samunohito
Copy link
Member Author

https://github.com/misskey-dev/misskey/blob/develop/cypress/support/commands.ts#L33-L35
↑をコメントアウトすると正常に動作するようになる。

https://developer.mozilla.org/ja/docs/Web/API/IDBFactory/deleteDatabase に記載のある 注: この機能は Web Worker 内で利用可能です が関係している?

@samunohito
Copy link
Member Author

samunohito commented Mar 20, 2024

以下推測

直近では #13591 にて上記箇所の修正が施されているが、これは直接的な原因ではないと考える。
元々は

cy.window(win => {
  win.indexedDB.deleteDatabase('keyval-store');
})

のような実装になっていたが、cy.window()にはラムダを引数に取る機能は存在しておらず、ただcy.window()を呼び出していただけだった。TypeScript化されたことにより正しく呼び出せていないことが表面化し(型エラーで)、現在の形に修正されたことで表出したのではないかと推測。

ただ…13591でもコメントされている通り、上手く行ったパターンもあるとのこと…
環境依存的なものもあるのかもしれない(自分のローカルではうまくいかなかった)

@lunaisnotaboy
Copy link

@dakkar you said that it still worked when you tested it locally, correct?

@dakkar
Copy link
Contributor

dakkar commented Mar 20, 2024

it did, yes

@dakkar
Copy link
Contributor

dakkar commented Mar 20, 2024

but I may have tested before "fixing" the call to deleteDatabase; I'll test again tomorrow

@dakkar
Copy link
Contributor

dakkar commented Mar 21, 2024

you're all correct, "fixing" the call to deleteDatabase made the tests fail

just delete the lines

cy.window().then(win => {
  win.indexedDB.deleteDatabase('keyval-store');
});

from cypress/support/commands.ts and everything works again.

Apologies for breaking your build!

@lunaisnotaboy
Copy link

@dakkar that's not a good thing. that's what resets the client's IndexedDB between tests, so it can have a fresh start. another fix needs to be found. i'll see what i can do.

@dakkar
Copy link
Contributor

dakkar commented Mar 21, 2024

well, the code before we "fixed" it, was passing a lambda to the cy.window() function, which completely ignored it, so the IndexedDB was never reset

apparently the tests actually rely on that?

@lunaisnotaboy
Copy link

hmmm...

then i guess deleting the lines would do it

@lunaisnotaboy
Copy link

i don't get it... because the way we did it is exactly how we're supposed to do it...

@lunaisnotaboy
Copy link

related: cypress-io/cypress#1208

@tamaina
Copy link
Contributor

tamaina commented Mar 21, 2024

とりあえずidb-proxyでlocalStorageを強制するのはどうかしら

@lunaisnotaboy
Copy link

とりあえずidb-proxyでlocalStorageを強制するのはどうかしら

Maybe... that could work.

@anatawa12
Copy link
Member

https://developer.mozilla.org/ja/docs/Web/API/IDBFactory/deleteDatabase に記載のある 注: この機能は Web Worker 内で利用可能です が関係している?

この注は数多くのweb APIがweb worker内で使えないのに対して、このAPIが(比較的)例外的に使えるAPIであることを示しているだけで非web worker内で使えないという話ではないと思います。

@anatawa12
Copy link
Member

少し調査してみたところ、chromeのバグに起因してる可能性がありそうでした。

まず現状の動作を確認したところ、なぜか win.indexedDB.deleteDatabase('keyval-store') が完了しない1ことがわかりました。
また、cypressの内部のmisskeyがローディングで止まる原因を調査したところこちらのawait iset('idb-test', 'test')内で呼ばれている indexedDB.openも完了しなくなっている模様でした。

また、firefox環境ではcypressが問題なく通ることも確認したため、chrome固有の動作であろうことがわかりました。

そして、おそらくcypressはiframecontentWindowを使ってる推測し、このcontentWindowを経由したindexedDBの操作周りにchromeにバグが有るのではないかと予測しました。

これを確認するために、以下のようなhtmlファイルを作成し、これをhttp経由でchromeで開いたところ、innerWindow.indexedDB.deleteDatabaseが正常に完了しない動作が再現できたため、chromeのバグであると考えました。
(firefoxでは問題なく完了することも確認しました)

<!doctype html>
<iframe></iframe>
<script>
	const search = new URLSearchParams(location.search);
	if (search.has("inner")) {
		// this is the inner iframe
	} else {
		// this is the outer iframe
		const iframe = document.querySelector("iframe");
		iframe.src = location.href + "?inner";
		const innerWindow = iframe.contentWindow;

		const request = innerWindow.indexedDB.deleteDatabase("test-db");
		request.onsuccess = () => {
			console.log("Database deleted");
		};
		request.onerror = () => {
			console.error("Error deleting database");
		};
		request.onblocked = () => {
			console.error("Blocked deleting database");
		};
		request.onupgradeneeded = () => {
			console.error("Upgrade needed deleting database");
		};
		console.log(request);
	}
</script>

Footnotes

  1. いくらまっても戻り値のIDBRequest.readyState'done'にならず、またIDBRequest.onsuccess, IDBRequest.onerror, IDBOpenDBRequest.onblocked, IDBOpenDBRequest.upgradeneededイベントのいずれも発火しない。

@tamaina
Copy link
Contributor

tamaina commented Apr 13, 2024

indexedDB.deleteDatabaseが正常に完了しない

これChromiumのバグトラッカーないかしら(前から気になってはいた

@anatawa12
Copy link
Member

https://issues.chromium.org/issues?q=indexedDB%20deleteDatabase https://issues.chromium.org/issues?q=indexedDB%20iframe で調べた感じ、https://issues.chromium.org/issues/40827490 が関係あるかもしれないくらいでそのものっぽいのは見当たらなかった

@github-project-automation github-project-automation bot moved this from Triage to Done in [BUG TRACKER] Dev Apr 13, 2024
@samunohito samunohito added the 🛠️Dev Development of Misskey itself label Nov 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️bug? This might be a bug 🛠️Dev Development of Misskey itself
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants