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

Test enhancements / after electron port #3459

Open
jessfraz opened this issue Aug 15, 2024 · 8 comments
Open

Test enhancements / after electron port #3459

jessfraz opened this issue Aug 15, 2024 · 8 comments
Labels
desktop-app Issues from the desktop app. performance tests Pull requests that update or improve our test suite

Comments

@jessfraz
Copy link
Contributor

jessfraz commented Aug 15, 2024

Some questions for @ryanrosello-og when he’s back and after initial electron merge also cc @lf94 for anything I’m missing

  1. vscode is electron and opens really fast, electron has great docs on how vscode achieved this: https://electronjs.org/docs/latest/tutorial/performance but how do we ensure on every PR we aren’t making the app open slower and that just general interactions aren’t slower
  2. how can we test double clicking a file in the os finder and it opening in the app (and performance test speed to open here as well)
  3. how can we test opening a file via double click where we have to accept a native os dialog for perms (we def want to test this code path)
  4. How can we test core dump from the desktop / browser without spamming our repo w core dump issues
  5. How can we test the auto updater
    ̶6̶.̶ ̶h̶t̶t̶p̶s̶:̶/̶/̶g̶i̶t̶h̶u̶b̶.̶c̶o̶m̶/̶K̶i̶t̶t̶y̶C̶A̶D̶/̶m̶o̶d̶e̶l̶i̶n̶g̶-̶a̶p̶p̶/̶p̶u̶l̶l̶/̶3̶3̶1̶5̶/̶c̶o̶m̶m̶i̶t̶s̶/̶b̶4̶e̶0̶b̶d̶0̶3̶8̶e̶9̶5̶b̶2̶5̶a̶c̶3̶2̶c̶4̶b̶f̶b̶2̶3̶a̶c̶4̶b̶0̶0̶c̶8̶7̶0̶a̶d̶2̶3̶ ̶c̶o̶p̶i̶e̶s̶ ̶d̶e̶v̶ ̶e̶n̶v̶ ̶v̶a̶r̶s̶ ̶i̶n̶t̶o̶ ̶̶.̶e̶n̶v̶.̶p̶r̶o̶d̶u̶c̶t̶i̶o̶n̶̶ ̶t̶h̶e̶r̶e̶ ̶m̶u̶s̶t̶ ̶b̶e̶ ̶a̶ ̶b̶e̶t̶t̶e̶r̶ ̶w̶a̶y̶ ̶t̶o̶ ̶g̶e̶t̶ ̶̶y̶a̶r̶n̶ ̶e̶l̶e̶c̶t̶r̶o̶n̶:̶p̶a̶c̶k̶a̶g̶e̶̶ ̶t̶o̶ ̶e̶i̶t̶h̶e̶r̶ ̶b̶u̶i̶l̶d̶ ̶o̶r̶ ̶r̶u̶n̶ ̶i̶n̶ ̶d̶e̶v̶ ̶m̶o̶d̶e̶ ̶s̶o̶ ̶t̶h̶a̶t̶ ̶i̶t̶ ̶u̶s̶e̶s̶ ̶t̶h̶e̶ ̶d̶e̶v̶ ̶e̶n̶v̶ ̶v̶a̶r̶s̶.̶

Also in relation to #1 in the list we don’t even have metrics on perf today but we will want them so we can make it faster

@jessfraz jessfraz added performance tests Pull requests that update or improve our test suite labels Aug 15, 2024
@ryanrosello-og
Copy link
Contributor

Howdy @jessfraz , sorry for the late reply:

  1. i can think of two options:
    (a) add a timer at the start of setupElectron and let it load the app, once an element on page appears .e.g Create New project button, stop the timer and compare against some threshold value. If the time taken is more than the threshold value, fail the test
    (b) add a secret hidden element on the page which tracks the app load time, we can then use playwright to query the value of this element and compare against some threshold value.

2 and 3. Unfortunately, this is going well beyond Playwrights' capabilities since it can really only deal with the browser. Perhaps another tool can be utilized? I doubt that it would be open source so we'd probably need to engage with some tool vendor and then have all of it integrated into GH workflow somehow

  1. Do we only want to do this for failing tests and can we control which folder the dump goes into? If so, we can have Playwright examine the contents of the folder/files, delete it upon completion

  2. hmm, tricky. We'd need to install initial packaged version of the app first and then run the updater on top if it. I guess we'd need to do this for all support OS and we need to take into considering which version the user is updating from

  3. I've used cross-env in my past life:

smth like this?

"electron:package": "cross-env SOME_VAR=SOME_VALUE electron-forge package",

"electron:package:dev": "cross-env SOME_VAR=SOME_OTHER_VALUE electron-forge package",

And enable dotenv within the playwright config file

@lf94
Copy link
Contributor

lf94 commented Aug 16, 2024

b4e0bd0 copies dev env vars into .env.production there must be a better way to get yarn electron:package to either build or run in dev mode so that it uses the dev env vars.

Why not NODE_ENV=production to load the .env.production


I think you covered everything - yeah these all seem like hella hard to test outside of playwright :D.

@jessfraz
Copy link
Contributor Author

oh number 6 is irrelevant i removed that

@jessfraz
Copy link
Contributor Author

so right now its working fine without it!

@jessfraz
Copy link
Contributor Author

also for coredump i just mean when you hit the file a bug button it works and opens up github thats all, nothing too fancy but idk how to test for external windows and shit

@jessfraz jessfraz pinned this issue Aug 19, 2024
@ryanrosello-og
Copy link
Contributor

also for coredump i just mean when you hit the file a bug button it works and opens up github thats all, nothing too fancy but idk how to test for external windows and shit

no worries, I think Kurt had a similar issue with one of his tests, I will have a poke around and see what is doable

@jessfraz
Copy link
Contributor Author

#3588 is relevant

@jessfraz jessfraz added the desktop-app Issues from the desktop app. label Aug 21, 2024
@s1hofmann
Copy link

Hi @ryanrosello-og, I saw s post from @jessfraz online and wanted to chime in here:

  1. how can we test double clicking a file in the os finder and it opening in the app (and performance test speed to open here as well)
  2. how can we test opening a file via double click where we have to accept a native os dialog for perms (we def want to test this code path)

I could offer help with nut.js to tackle this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop-app Issues from the desktop app. performance tests Pull requests that update or improve our test suite
Projects
None yet
Development

No branches or pull requests

4 participants