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.

Sign up for GitHub

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

Jump to bottom

[Code] fix history functional test #37332

Merged
merged 2 commits into from
Jun 3, 2019
Merged

[Code] fix history functional test #37332

merged 2 commits into from
Jun 3, 2019

Conversation

WangQianliang
Copy link
Contributor

Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@WangQianliang
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@WangQianliang
Copy link
Contributor Author

fix https://github.com/elastic/code/issues/1184

@WangQianliang
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@WangQianliang
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@WangQianliang
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@WangQianliang WangQianliang changed the title [WIP][Code] fix history functional test [Code] fix history functional test May 30, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@WangQianliang WangQianliang requested a review from mw-ding May 31, 2019 02:01
@WangQianliang
Copy link
Contributor Author

retest

mw-ding
mw-ding reviewed May 31, 2019
View reviewed changes
x-pack/test/functional/apps/code/history.ts
const tabText = await (await find.byCssSelector('.euiTab-isSelected')).getVisibleText();
expect(tabText).to.equal('Structure');
// if structure tree tab is active, file tree tab's `data-test-subj` would be `codeFileTreeTab`
expect(testSubjects.exists('codeFileTreeTab')).to.be.ok();
Copy link
Contributor

Choose a reason for hiding this comment

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

should we change to codeStructureTreeTabActive which is more readable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

change to codeStructureTreeTabActive would affect explore_repository test, can we leave it for now

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@elasticmachine
Copy link
Contributor

💔 Build Failed

@WangQianliang WangQianliang requested a review from mw-ding May 31, 2019 04:18
@WangQianliang
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@mw-ding
Copy link
Contributor

mw-ding commented Jun 1, 2019

can we try to rerun this test with it enabled multiple times to make sure it won't be flaky?

mw-ding
mw-ding reviewed Jun 1, 2019
View reviewed changes
Copy link
Contributor

@mw-ding mw-ding left a comment

Choose a reason for hiding this comment

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

see my comments above

@WangQianliang
Copy link
Contributor Author

can we try to rerun this test with it enabled multiple times to make sure it won't be flaky?

It ran hundreds of times before, and the results were good(some failures were caused by other tests)

@WangQianliang WangQianliang requested a review from mw-ding June 1, 2019 08:36
@WangQianliang
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

mw-ding
mw-ding approved these changes Jun 3, 2019
View reviewed changes
Copy link
Contributor

@mw-ding mw-ding left a comment

Choose a reason for hiding this comment

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

Let's just leave this PR in master, without backporting them to 7.x or 7.2

mw-ding
mw-ding reviewed Jun 3, 2019
View reviewed changes
x-pack/test/functional/apps/code/explore_repository.ts
// expect(await testSubjects.exists('codeSourceViewer')).to.be(true);
// });
// });
it('Click file/directory on the right panel', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think in this PR, the explore-repository test is skipped, so do we need to keep the changes in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's keep the change since we are gonna uncomment this any way

@WangQianliang WangQianliang merged commit 570135b into elastic:master Jun 3, 2019
@WangQianliang WangQianliang deleted the code-fix-history-functional-test branch June 3, 2019 07:21
@WangQianliang
Copy link
Contributor Author

backport pending for some tests are flaky

@spalger spalger mentioned this pull request Jul 2, 2019
Closed
@WangQianliang WangQianliang mentioned this pull request Jul 3, 2019
Merged
WangQianliang pushed a commit that referenced this pull request Jul 3, 2019
@WangQianliang
[Code] fix history functional test (#37332)
a024568 
* enable history functional test and fix flaky test
WangQianliang pushed a commit that referenced this pull request Jul 4, 2019
[Code] fix history functional test (#37332) (#40269)
359c9ad 
* enable history functional test and fix flaky test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mw-ding mw-ding mw-ding approved these changes

Assignees
No one assigned
Labels
backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@WangQianliang @elasticmachine @mw-ding