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

Fix atom-application tests #19124

Merged
merged 1 commit into from Apr 9, 2019

Conversation

Projects
None yet
1 participant
@rafeca
Copy link
Contributor

commented Apr 9, 2019

When closing a window with a file that does not exist, Atom opens a dialog asking the users if they want to save the changes. This dialog prevents the tests from finishing correctly (as seen in the CI builds).

I still don't know why this test can ever pass (currently it always fails on my OSX machine).

This should fix #18993

Fix atom-application tests
When closing a window with a file that does not exist, Atom opens a
dialog asking the users if they want to save the changes. This dialog
prevented the tests from finishing correctly.
@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

It seems that this test passes whenever the following trace is shown on the logs:

Object has been destroyed
Error: Object has been destroyed
    at WebContents.send (/Users/rafeca/github/atom/atom/out/Atom Dev.app/Contents/Resources/electron.asar/browser/api/web-contents.js:101:15)
    at EventEmitter.exports.on (/Users/rafeca/github/atom/atom/src/ipc-helpers.js:41:18)
    at <anonymous>:null:null

So probably a different error is causing Atom to crash and that makes this test pass 🤦‍♂️

This is related to #19089

@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

After further investigation it looks like the error mentioned above is unrelated with this flakiness: the problem is that the test does not wait until Atom has been fully loaded so if Atom loads faster than expected the confirm dialog will be shown and the test will fail.

@rafeca

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

Gonna land this one and continue investigating the root issue, which is maybe causing other failures

@rafeca rafeca merged commit d6f06f6 into master Apr 9, 2019

2 checks passed

Atom Pull Requests #20190409.1 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@rafeca rafeca deleted the fix-application-test branch Apr 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.