Skip to content

Making it always wait the same amount#267

Merged
igor-ribeiiro merged 3 commits intoOfficeDev:masterfrom
igor-ribeiiro:flaky_tests
Apr 6, 2021
Merged

Making it always wait the same amount#267
igor-ribeiiro merged 3 commits intoOfficeDev:masterfrom
igor-ribeiiro:flaky_tests

Conversation

@igor-ribeiiro
Copy link
Copy Markdown
Contributor

Making test always wait 8000 regardless of flaky tests to make tests more reliable.

@igor-ribeiiro igor-ribeiiro self-assigned this Apr 6, 2021
@igor-ribeiiro igor-ribeiiro requested a review from a team as a code owner April 6, 2021 19:10
@akrantz
Copy link
Copy Markdown
Contributor

akrantz commented Apr 6, 2021

8 seconds is a really long time. Could it be smaller?

I'm OK with increasing the value some if that is causing the tests to be flaky, but it would be nice if there was a reliable way to know when the function has returned. One way would be to use an Office.js event to know when the cell value was changed. A less elegant way would be to get the value in a loop until changed (or timed out).

@igor-ribeiiro
Copy link
Copy Markdown
Contributor Author

I'm OK with increasing the value some if that is causing the tests to be flaky, but it would be nice if there was a reliable way to know when the function has returned. One way would be to use an Office.js event to know when the cell value was changed. A less elegant way would be to get the value in a loop until changed (or timed out).

Yeah, just waiting around isn't the best, but it was the way it was being done already. A loop solution could give better times on success cases, but fail cases would be the same.
Given that we just have two tests with these sleeps, I guess it shouldn't be that bad to just wait on them.

@akrantz
Copy link
Copy Markdown
Contributor

akrantz commented Apr 6, 2021

How about we make it 5 seconds? That should be more than enough time for any platform.

akrantz
akrantz previously approved these changes Apr 6, 2021
@TCourtneyOwen
Copy link
Copy Markdown
Contributor

TCourtneyOwen commented Apr 6, 2021

That's great to see that this fixes it. Also, I have added the enable-edge-webview artifact to our VM deployment script so we shouldn't hit the webivew hang problem moving forward

TCourtneyOwen
TCourtneyOwen previously approved these changes Apr 6, 2021
@igor-ribeiiro igor-ribeiiro dismissed stale reviews from TCourtneyOwen and akrantz via 7006329 April 6, 2021 19:42
@igor-ribeiiro
Copy link
Copy Markdown
Contributor Author

How about we make it 5 seconds? That should be more than enough time for any platform.

Just did!

@igor-ribeiiro igor-ribeiiro merged commit dd7151c into OfficeDev:master Apr 6, 2021
@igor-ribeiiro igor-ribeiiro deleted the flaky_tests branch April 6, 2021 20:16
igor-ribeiiro added a commit that referenced this pull request Apr 12, 2021
* Fixing breakpoints for VSCode

* Making HTML not get auto deleted (#266)

Had to override the default behavior of the CleanWebpackPlugin. It normally deletes all files creates by other plugins that are not webpack.
This is the case for out HTML, css and one .xml file. So I made it just clean the files before compiling and not after.

Now you can run npm run build followed by npm start and the project would work.

* Making it always wait the same amount (#267)

Making test always wait 5000 regardless of flaky tests to make tests more reliable.
igor-ribeiiro added a commit that referenced this pull request Apr 12, 2021
* Fixing breakpoints for VSCode

* Making HTML not get auto deleted (#266)

Had to override the default behavior of the CleanWebpackPlugin. It normally deletes all files creates by other plugins that are not webpack.
This is the case for out HTML, css and one .xml file. So I made it just clean the files before compiling and not after.

Now you can run npm run build followed by npm start and the project would work.

* Making it always wait the same amount (#267)

Making test always wait 5000 regardless of flaky tests to make tests more reliable.
millerds pushed a commit that referenced this pull request Oct 2, 2024
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.

3 participants