[Code] Fix code explore repository functional test (#36157) (#38259)

* Revert "Revert "[Code] set loading false if do not need fetch tree (#36083)""

This reverts commit 6a013cb82e.

* fix flaky `explore_repository` test
This commit is contained in:
WangQianliang 2019-06-07 07:13:42 +08:00 committed by Fuyao Zhao
parent 4e500d982c
commit 47b545d443
9 changed files with 121 additions and 137 deletions

View file

@ -44,7 +44,6 @@ test('render correctly', () => {
location={location}
closeTreePath={mockFunction}
openTreePath={mockFunction}
fetchRepoTree={mockFunction}
/>
)
.toJSON();

View file

@ -11,7 +11,7 @@ import classes from 'classnames';
import { connect } from 'react-redux';
import { RouteComponentProps, withRouter } from 'react-router-dom';
import { FileTree as Tree, FileTreeItemType } from '../../../model';
import { closeTreePath, fetchRepoTree, FetchRepoTreePayload, openTreePath } from '../../actions';
import { closeTreePath, openTreePath } from '../../actions';
import { EuiSideNavItem, MainRouteParams, PathTypes } from '../../common/types';
import { RootState } from '../../reducers';
import { encodeRevisionString } from '../../utils/url';
@ -20,9 +20,7 @@ interface Props extends RouteComponentProps<MainRouteParams> {
node?: Tree;
closeTreePath: (paths: string) => void;
openTreePath: (paths: string) => void;
fetchRepoTree: (p: FetchRepoTreePayload) => void;
openedPaths: string[];
treeLoading?: boolean;
}
export class CodeFileTree extends React.Component<Props> {
@ -33,16 +31,6 @@ export class CodeFileTree extends React.Component<Props> {
}
}
public fetchTree(path = '', isDir: boolean) {
const { resource, org, repo, revision } = this.props.match.params;
this.props.fetchRepoTree({
uri: `${resource}/${org}/${repo}`,
revision,
path: path || '',
isDir,
});
}
public onClick = (node: Tree) => {
const { resource, org, repo, revision, path } = this.props.match.params;
if (!(path === node.path)) {
@ -257,11 +245,9 @@ export class CodeFileTree extends React.Component<Props> {
const mapStateToProps = (state: RootState) => ({
node: state.file.tree,
openedPaths: state.file.openedPaths,
treeLoading: state.file.rootFileTreeLoading,
});
const mapDispatchToProps = {
fetchRepoTree,
closeTreePath,
openTreePath,
};

View file

@ -53,8 +53,8 @@ interface Props extends RouteComponentProps<MainRouteParams> {
onSearchScopeChanged: (s: SearchScope) => void;
repoScope: string[];
notFoundDirs: string[];
fileTreeLoadingPaths: string[];
searchOptions: SearchOptions;
fileTreeLoading: boolean;
query: string;
}
const LANG_MD = 'markdown';
@ -266,7 +266,7 @@ class CodeContent extends React.PureComponent<Props> {
}
public renderContent() {
const { file, match, tree, fileTreeLoading, isNotFound, notFoundDirs } = this.props;
const { file, match, tree, fileTreeLoadingPaths, isNotFound, notFoundDirs } = this.props;
const { path, pathType, resource, org, repo, revision } = match.params;
if (isNotFound || notFoundDirs.includes(path || '')) {
return <NotFound />;
@ -281,7 +281,7 @@ class CodeContent extends React.PureComponent<Props> {
const node = this.findNode(path ? path.split('/') : [], tree);
return (
<div className="codeContainer__directoryView">
<Directory node={node} loading={fileTreeLoading} />
<Directory node={node} loading={fileTreeLoadingPaths.includes(path)} />
<CommitHistory
repoUri={repoUri}
header={
@ -390,7 +390,7 @@ const mapStateToProps = (state: RootState) => ({
notFoundDirs: state.file.notFoundDirs,
file: state.file.file,
tree: state.file.tree,
fileTreeLoading: state.file.fileTreeLoading,
fileTreeLoadingPaths: state.file.fileTreeLoadingPaths,
currentTree: currentTreeSelector(state),
branches: state.file.branches,
hasMoreCommits: hasMoreCommitsSelector(state),

View file

@ -74,7 +74,7 @@ class CodeMain extends React.Component<Props> {
}
const mapStateToProps = (state: RootState) => ({
loadingFileTree: state.file.rootFileTreeLoading,
loadingFileTree: state.file.fileTreeLoadingPaths.includes(''),
loadingStructureTree: state.symbol.loading,
hasStructure: structureSelector(state).length > 0 && !state.symbol.error,
languageServerInitializing: state.symbol.languageServerInitializing,

View file

@ -36,8 +36,6 @@ import {
export interface FileState {
tree: FileTree;
fileTreeLoading: boolean;
rootFileTreeLoading: boolean;
openedPaths: string[];
// store not found directory as an array to calculate `notFound` flag by finding whether path is in this array
notFoundDirs: string[];
@ -51,6 +49,7 @@ export interface FileState {
currentPath: string;
loadingCommits: boolean;
commitsFullyLoaded: { [path: string]: boolean };
fileTreeLoadingPaths: string[];
}
const initialState: FileState = {
@ -62,8 +61,7 @@ const initialState: FileState = {
},
openedPaths: [],
notFoundDirs: [],
rootFileTreeLoading: true,
fileTreeLoading: false,
fileTreeLoadingPaths: [''],
branches: [],
tags: [],
commits: [],
@ -115,13 +113,15 @@ export const file = handleActions(
[String(fetchRepoTree)]: (state: FileState, action: any) =>
produce(state, draft => {
draft.currentPath = action.payload.path;
draft.fileTreeLoading = true;
// @ts-ignore
draft.fileTreeLoadingPaths.push(action.payload!.path);
}),
[String(fetchRepoTreeSuccess)]: (state: FileState, action: Action<RepoTreePayload>) =>
produce<FileState>(state, (draft: FileState) => {
draft.fileTreeLoading = false;
draft.rootFileTreeLoading = false;
draft.notFoundDirs = draft.notFoundDirs.filter(dir => dir !== action.payload!.path);
draft.fileTreeLoadingPaths = draft.fileTreeLoadingPaths.filter(
p => p !== action.payload!.path && p !== ''
);
const { tree, path, withParents } = action.payload!;
if (withParents || path === '/' || path === '') {
draft.tree = mergeNode(draft.tree, tree);
@ -142,12 +142,12 @@ export const file = handleActions(
}),
[String(fetchRootRepoTreeSuccess)]: (state: FileState, action: Action<any>) =>
produce<FileState>(state, (draft: FileState) => {
draft.rootFileTreeLoading = false;
draft.fileTreeLoadingPaths = draft.fileTreeLoadingPaths.filter(p => p !== '/' && p !== '');
draft.tree = mergeNode(draft.tree, action.payload!);
}),
[String(fetchRootRepoTreeFailed)]: (state: FileState, action: Action<any>) =>
produce<FileState>(state, (draft: FileState) => {
draft.rootFileTreeLoading = false;
draft.fileTreeLoadingPaths = draft.fileTreeLoadingPaths.filter(p => p !== '/' && p !== '');
}),
[String(dirNotFound)]: (state: FileState, action: any) =>
produce<FileState>(state, (draft: FileState) => {
@ -158,10 +158,11 @@ export const file = handleActions(
draft.tree = initialState.tree;
draft.openedPaths = initialState.openedPaths;
}),
[String(fetchRepoTreeFailed)]: (state: FileState) =>
[String(fetchRepoTreeFailed)]: (state: FileState, action: Action<any>) =>
produce(state, draft => {
draft.fileTreeLoading = false;
draft.rootFileTreeLoading = false;
draft.fileTreeLoadingPaths = draft.fileTreeLoadingPaths.filter(
p => p !== action.payload!.path && p !== ''
);
}),
[String(openTreePath)]: (state: FileState, action: Action<any>) =>
produce<FileState>(state, (draft: FileState) => {

View file

@ -43,6 +43,7 @@ import {
refUrlSelector,
repoScopeSelector,
urlQueryStringSelector,
createTreeSelector,
} from '../selectors';
import { history } from '../utils/url';
import { mainRoutePattern } from './patterns';
@ -203,15 +204,27 @@ function* handleMainRouteChange(action: Action<Match>) {
.slice(0, -1)
.join('/');
yield put(openTreePath(openPath || ''));
yield put(
fetchRepoTree({
uri: repoUri,
revision,
path: file || '',
parents: getPathOfTree(tree, (file || '').split('/')) === null,
isDir,
})
);
function isTreeLoaded(isDirectory: boolean, targetTree: FileTree | null) {
if (!isDirectory) {
return !!targetTree;
} else if (!targetTree) {
return false;
} else {
return targetTree.children && targetTree.children.length > 0;
}
}
const targetTree: FileTree | null = yield select(createTreeSelector(file || ''));
if (!isTreeLoaded(isDir, targetTree)) {
yield put(
fetchRepoTree({
uri: repoUri,
revision,
path: file || '',
parents: getPathOfTree(tree, (file || '').split('/')) === null,
isDir,
})
);
}
const uri = toCanonicalUrl({
repoUri,
file,

View file

@ -10,7 +10,6 @@ import { kfetch } from 'ui/kfetch';
import Url from 'url';
import { call, put, select, takeEvery, takeLatest } from 'redux-saga/effects';
import { FileTree } from '../../model';
import {
fetchDirectory,
fetchDirectoryFailed,
@ -45,51 +44,37 @@ import {
dirNotFound,
} from '../actions';
import { RootState } from '../reducers';
import { treeCommitsSelector, createTreeSelector } from '../selectors';
import { treeCommitsSelector } from '../selectors';
import { repoRoutePattern } from './patterns';
import { FileTree } from '../../model';
function* handleFetchRepoTree(action: Action<FetchRepoTreePayload>) {
try {
const { uri, revision, path, parents, isDir } = action.payload!;
if (path && isDir) {
const tree = yield select(createTreeSelector(path));
if (tree) {
const { children } = tree;
// do not request file tree if this tree exists and its children are not empty
if (!children || children.length === 0) {
yield call(fetchPath, { uri, revision, path, parents, isDir });
}
const tree = yield call(requestRepoTree, action.payload!);
(tree.children || []).sort((a: FileTree, b: FileTree) => {
const typeDiff = a.type - b.type;
if (typeDiff === 0) {
return a.name > b.name ? 1 : -1;
} else {
yield call(fetchPath, { uri, revision, path, parents, isDir });
return -typeDiff;
}
} else {
yield call(fetchPath, action.payload!);
}
});
tree.repoUri = action.payload!.uri;
yield put(
fetchRepoTreeSuccess({
tree,
path: action.payload!.path,
withParents: action.payload!.parents,
})
);
} catch (err) {
if (action.payload!.isDir && err.body && err.body.statusCode === 404) {
yield put(dirNotFound(action.payload!.path));
}
yield put(fetchRepoTreeFailed(err));
yield put(fetchRepoTreeFailed({ ...err, path: action.payload!.path }));
}
}
function* fetchPath(payload: FetchRepoTreePayload) {
const update: FileTree = yield call(requestRepoTree, payload);
(update.children || []).sort((a, b) => {
const typeDiff = a.type - b.type;
if (typeDiff === 0) {
return a.name > b.name ? 1 : -1;
} else {
return -typeDiff;
}
});
update.repoUri = payload.uri;
yield put(
fetchRepoTreeSuccess({ tree: update, path: payload.path, withParents: payload.parents })
);
return update;
}
interface FileTreeQuery {
parents?: boolean;
limit: number;

View file

@ -133,7 +133,9 @@ function* handleReposStatusLoaded(action: Action<any>) {
// Load current repository status on main page
const currentUri = yield select(repoUriSelector);
const status = allStatuses[currentUri];
yield triggerPollRepoStatus(status.state, currentUri);
if (status) {
yield triggerPollRepoStatus(status.state, currentUri);
}
}
}

View file

@ -8,7 +8,7 @@ import expect from '@kbn/expect';
import { TestInvoker } from './lib/types';
// eslint-disable-next-line import/no-default-export
export default function exploreRepositoryFunctonalTests({
export default function exploreRepositoryFunctionalTests({
getService,
getPageObjects,
}: TestInvoker) {
@ -23,8 +23,7 @@ export default function exploreRepositoryFunctonalTests({
const FIND_TIME = config.get('timeouts.find');
// FLAKY https://github.com/elastic/kibana/issues/35944
describe.skip('Explore Repository', () => {
describe('Explore Repository', () => {
describe('Explore a repository', () => {
const repositoryListSelector = 'codeRepositoryList codeRepositoryItem';
@ -228,72 +227,70 @@ export default function exploreRepositoryFunctonalTests({
});
});
// TODO(qianliang): blocked by https://github.com/elastic/code/issues/1163
// it('Click file/directory on the right panel', async () => {
// log.debug('Click file/directory on the right panel');
it('Click file/directory on the right panel', async () => {
log.debug('Click file/directory on the right panel');
// // Wait the file tree to be rendered and click the 'src' folder on the file tree.
// await retry.try(async () => {
// expect(await testSubjects.exists('codeFileExplorerNode-src')).to.be(true);
// });
// Wait the file tree to be rendered and click the 'src' folder on the file tree.
await retry.try(async () => {
expect(await testSubjects.exists('codeFileExplorerNode-src')).to.be(true);
});
// await testSubjects.click('codeFileExplorerNode-src');
// await retry.try(async () => {
// expect(await testSubjects.exists('codeFileExplorerNode-models')).to.be(true);
// });
await testSubjects.click('codeFileExplorerNode-src');
await retry.try(async () => {
expect(await testSubjects.exists('codeFileExplorerNode-models')).to.be(true);
});
// await testSubjects.click('codeFileExplorerNode-models');
// // Then the 'models' folder on the file tree.
// await retry.try(async () => {
// expect(await testSubjects.exists('codeFileExplorerNode-User.ts')).to.be(true);
// });
await testSubjects.click('codeFileExplorerNode-models');
// Then the 'models' folder on the file tree.
await retry.try(async () => {
expect(await testSubjects.exists('codeFileExplorerNode-User.ts')).to.be(true);
});
// await testSubjects.click('codeFileExplorerNode-User.ts');
// // Then the 'User.ts' file on the file tree.
// await retry.try(async () => {
// expect(await testSubjects.exists('codeSourceViewer')).to.be(true);
// });
// });
await testSubjects.click('codeFileExplorerNode-User.ts');
// Then the 'User.ts' file on the file tree.
await retry.try(async () => {
expect(await testSubjects.exists('codeSourceViewer')).to.be(true);
});
});
// TODO(qianliang): blocked by https://github.com/elastic/code/issues/1163
// it('Navigate source file via structure tree', async () => {
// log.debug('Navigate source file via structure tree');
// // Wait the file tree to be rendered and click the 'src' folder on the file tree.
// await retry.try(async () => {
// expect(await testSubjects.exists('codeFileExplorerNode-src')).to.be(true);
// });
it('Navigate source file via structure tree', async () => {
log.debug('Navigate source file via structure tree');
// Wait the file tree to be rendered and click the 'src' folder on the file tree.
await retry.try(async () => {
expect(await testSubjects.exists('codeFileExplorerNode-src')).to.be(true);
});
// await testSubjects.click('codeFileExplorerNode-src');
// await retry.try(async () => {
// expect(await testSubjects.exists('codeFileExplorerNode-models')).to.be(true);
// });
await testSubjects.click('codeFileExplorerNode-src');
await retry.try(async () => {
expect(await testSubjects.exists('codeFileExplorerNode-models')).to.be(true);
});
// await testSubjects.click('codeFileExplorerNode-models');
// // Then the 'models' folder on the file tree.
// await retry.try(async () => {
// expect(await testSubjects.exists('codeFileExplorerNode-User.ts')).to.be(true);
// });
await testSubjects.click('codeFileExplorerNode-models');
// Then the 'models' folder on the file tree.
await retry.try(async () => {
expect(await testSubjects.exists('codeFileExplorerNode-User.ts')).to.be(true);
});
// await testSubjects.click('codeFileExplorerNode-User.ts');
// // Then the 'User.ts' file on the file tree.
// await retry.try(async () => {
// expect(await testSubjects.exists('codeSourceViewer')).to.be(true);
// expect(await testSubjects.exists('codeStructureTreeTab')).to.be(true);
// });
await testSubjects.click('codeFileExplorerNode-User.ts');
// Then the 'User.ts' file on the file tree.
await retry.try(async () => {
expect(await testSubjects.exists('codeSourceViewer')).to.be(true);
expect(await testSubjects.exists('codeStructureTreeTab')).to.be(true);
});
// // Click the structure tree tab
// await testSubjects.click('codeStructureTreeTab');
// await retry.tryForTime(300000, async () => {
// expect(await testSubjects.exists('codeStructureTreeNode-User')).to.be(true);
// Click the structure tree tab
await testSubjects.click('codeStructureTreeTab');
await retry.tryForTime(300000, async () => {
expect(await testSubjects.exists('codeStructureTreeNode-User')).to.be(true);
// await testSubjects.click('codeStructureTreeNode-User');
// await retry.tryForTime(120000, async () => {
// const currentUrl: string = await browser.getCurrentUrl();
// log.info(`Jump to url: ${currentUrl}`);
// expect(currentUrl.indexOf('src/models/User.ts!L92:6') > 0).to.be(true);
// });
// });
// });
await testSubjects.click('codeStructureTreeNode-User');
await retry.tryForTime(120000, async () => {
const currentUrl: string = await browser.getCurrentUrl();
log.info(`Jump to url: ${currentUrl}`);
expect(currentUrl.indexOf('src/models/User.ts!L92:6') > 0).to.be(true);
});
});
});
it('goes to a repository which does not exist should render the 404 error page', async () => {
log.debug('it goes to a repository which does not exist');
@ -302,9 +299,10 @@ export default function exploreRepositoryFunctonalTests({
await browser.get(url);
await retry.try(async () => {
const currentUrl: string = await browser.getCurrentUrl();
expect(currentUrl.indexOf(notExistRepoUri)).to.greaterThan(0);
// should redirect to main page
expect(currentUrl.indexOf(`${notExistRepoUri}/tree/master`)).to.greaterThan(0);
});
await retry.try(async () => {
await retry.tryForTime(5000, async () => {
expect(await testSubjects.exists('codeNotFoundErrorPage')).ok();
});
});