-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Code] bundle git repo for functional test #44547
Conversation
Pinging @elastic/code |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
packages/kbn-dev-utils/src/index.ts
Outdated
@@ -21,5 +21,5 @@ export { withProcRunner } from './proc_runner'; | |||
export { ToolingLog, ToolingLogTextWriter, pickLevelFromFlags } from './tooling_log'; | |||
export { createAbsolutePathSerializer } from './serializers'; | |||
export { CA_CERT_PATH, ES_KEY_PATH, ES_CERT_PATH } from './certs'; | |||
export { run, createFailError, createFlagError, combineErrors, isFailError } from './run'; | |||
export { run, createFailError, createFlagError, combineErrors, isFailError, Flags } from './run'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💔 Build Failed |
💔 Build Failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great, #44552 will be in shortly, then we can rebase this and I'd like to take another look (please re-request review).
}; | ||
|
||
const repoDir = (repoUri: string, kibanaDir: string) => { | ||
return `${kibanaDir}/data/code/repos/${repoUri}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use path.resolve()
here and in workspaceDir()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
import fs from 'fs'; | ||
import mkdirp from 'mkdirp'; | ||
import path from 'path'; | ||
import rimraf from 'rimraf'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I usually use del
because it does the same thing and supports promises.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
💔 Build Failed |
@spalger this should be good by now. can you take a second look? |
💚 Build Succeeded |
💚 Build Succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple minor nits, but LGTM
|
||
await retry.tryForTime(300000, async () => { | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just trying to differentiate 2 minor steps in this test case , which helps to be eyeballed.
|
||
await login(codeUser); | ||
await retry.tryForTime(5000, async () => { | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
Summary
Currently, Code functional tests rely on importing github repositories on the fly to clone the git repository as the source of truth. This external dependency could potentially result in some flakiness to the functional tests. In this PR, we bundled zipped git bare repositories into the kibana source code and extract these repos to the right code data path instead of cloning from github.com.
https://github.com/elastic/code/issues/1605
Regarding the runtime installation path of kibana, we can integrate with @spalger 's draft PR #44552 to make it more reasonable.
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers