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

chore: env. CLOSE_DEVTOOLS can enforce devtools closed during test #4988

Closed
wants to merge 2 commits into from

Conversation

odockal
Copy link
Contributor

@odockal odockal commented Nov 24, 2023

What does this PR do?

Introduces an option how to enforce podman desktop not to open dev tools window. Especially during testing.
This bring only a condition in packages/main/src/mainWindow.ts that checks for env. var. CLOSE_DEVTOOLS, if it is defined and is 'true', then browserWindow?.webContents.openDevTools(); is not called during application start up. This applies only in DEV mode which can be used mainly during yarn watch and during running the e2e tests. next pr will bring default setup of this env. var in tests/src/runner/podman-desktop-runner.ts during application start up. Resulting in a testing window without half of the space covered with dev tools.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

#4983 #4968

How to test this PR?

you can run tests without any change. yarn test:e2e:smoke. Check for the app during test run and verify that there is a dev tools opened. Then add cross-env CLOSE_DEVTOOLS=true at a beginning of a npm task test:e2e:smoke:ruin in package.json, task: `. If you then execute the tests, you should obeserve that dev tools were closed during testing.

@odockal odockal requested review from benoitf and a team as code owners November 24, 2023 18:19
@odockal odockal requested review from jeffmaury and cdrage and removed request for a team November 24, 2023 18:19
@@ -95,7 +95,9 @@ async function createWindow(): Promise<BrowserWindow> {
}

if (import.meta.env.DEV) {
browserWindow?.webContents.openDevTools();
if (!process.env.CLOSE_DEVTOOLS || process.env.CLOSE_DEVTOOLS !== 'true') {
Copy link
Contributor

Choose a reason for hiding this comment

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

That does not sound logic: I would rather have OPEN_DEVTOOLS=false to prevent opening DevTools. And test should be merged with previous test

@benoitf
Copy link
Collaborator

benoitf commented Nov 27, 2023

here I would opt-in for a Podman Desktop preference where in development mode, I can choose between : no dev tools, or all the options of devtools like the mode:

openDevTools has the parameter option:

options Object (optional)
mode string - Opens the devtools with specified dock state, can be left, right, bottom, undocked, detach. Defaults to last used dock state. In undocked mode it's possible to dock back. In detach mode it's not.

@odockal
Copy link
Contributor Author

odockal commented Nov 27, 2023

here I would opt-in for a Podman Desktop preference where in development mode, I can choose between : no dev tools, or all the options of devtools like the mode:

openDevTools has the parameter option:

options Object (optional)
mode string - Opens the devtools with specified dock state, can be left, right, bottom, undocked, detach. Defaults to last used dock state. In undocked mode it's possible to dock back. In detach mode it's not.

@benoitf Can you please clarify more about how this can be put into preferences and how it should work? Right now I am just assuming and doing something that will not show up on preferences since when in development mode, it is too late for e2e tests, and for yarn watch why should we be putting this into preferences as this option is already included in dev tools itself, developer can simply open dev tools and choose whatever suits him. right?

I can only see a benefit in using configuration properties as a way how to configure dev tools in dev mode on application start up but I am not sure how pick up two different options other than using env. vars. (Yes, @jeffmaury is right about wrong names).

Signed-off-by: Ondrej Dockal <odockal@redhat.com>
Signed-off-by: Ondrej Dockal <odockal@redhat.com>
@benoitf
Copy link
Collaborator

benoitf commented Nov 27, 2023

@benoitf Can you please clarify more about how this can be put into preferences and how it should work? Right now I am just assuming and doing something that will not show up on preferences since when in development mode, it is too late for e2e tests, and for yarn watch why should we be putting this into preferences as this option is already included in dev tools itself, developer can simply open dev tools and choose whatever suits him. right?

for example today, if I'm not interested to see devtools, I need to close the window each time I run it.
with a property I can say that I don't want to see it 'by default' on every startup and keep it only on demand.

so while it can fit with developer experience, I would rather add the setting as a setting rather than using an environment variable.

@odockal
Copy link
Contributor Author

odockal commented Nov 28, 2023

@benoitf Here is an issue to properly handle the problem: #5031

@benoitf
Copy link
Collaborator

benoitf commented Dec 18, 2023

superseded by #5280

@benoitf benoitf closed this Dec 18, 2023
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.

None yet

3 participants