Skip to content

test: add Monaco editor e2e regression coverage#3505

Open
msmithstubbs wants to merge 3 commits into
Logflare:mainfrom
msmithstubbs:test/monaco-e2e-regression
Open

test: add Monaco editor e2e regression coverage#3505
msmithstubbs wants to merge 3 commits into
Logflare:mainfrom
msmithstubbs:test/monaco-e2e-regression

Conversation

@msmithstubbs

@msmithstubbs msmithstubbs commented May 20, 2026

Copy link
Copy Markdown
Contributor

Adds baseline test for Monaco editor in preparation for refactor.

PR Stack

This is the first in a sequence of PRs that are intended to be merged in order.

See #3525

or search 🔍 is:pr O11Y-1543 sort:created-asc

Part of O11Y-1684 and O11Y-1543

Comment thread .github/workflows/elixir-ci.yml Outdated
assets/package-lock.json

- name: Install phoenix_test_jsdom Node dependencies
run: npm ci --prefix deps/phoenix_test_jsdom/priv

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense for these to be separated out to an elixir-web-ci workflow that only triggers on change in the assets or logflare_web directory.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise this essentially extends our CI for all changes even if its a minor change under the lib/logflare

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we can do a specific jsdom tag for selectively running these tests

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking with caching the npm install step would have minimal impact. I'll put it in a separate workflow for now. Would be good to have a strategy for where the jsdom tests fit in between playwright and LIveViewTest.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thought: we could hard code the list of events so it doesn't call node at compile time.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll have another look at it next week 🤞 , removing it from compile time should be possible, can likely do it at packaging side...
i've merged in your cookie persistence PR btw will cut a release for it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you like me to look into doing this? Or could move the JSDOM tests to a parallel PR? Be good to keep the monaco editor refactor moving forward.

@msmithstubbs msmithstubbs force-pushed the test/monaco-e2e-regression branch 6 times, most recently from 7297c88 to 702b79b Compare May 28, 2026 22:08
@msmithstubbs msmithstubbs force-pushed the test/monaco-e2e-regression branch from 702b79b to bf7a595 Compare May 29, 2026 04:27
@msmithstubbs msmithstubbs marked this pull request as ready for review May 29, 2026 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants