-
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
Run tests against an actual Kibana build #11237
Conversation
|
jenkins, test it |
Halle-f***ing-lujah! |
...kbnServerFlags, | ||
] | ||
}, | ||
|
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.
This is copypasta from testUIServer
. Is there a better way? Maybe reference the previous object and merge/extend it?
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.
It's your judgement call I'd say :). Something like the below isn't all that unwieldy.
const uiConfig = ...the thing...
testUIServer: {
...uiConfig,
cmd: binScript
},
testUIReleaseServer: {
...uiConfig,
cmd: binScript
},
Either way's fine IMHO
'esvm_shutdown:ui', | ||
'stop:testUIReleaseServer' | ||
]); | ||
|
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.
Also copypasta. Hm. I suppose one could pass in an array that's reused except for certain items, but I don't think it looks so great in this case. Would be super unclear indirection.
@@ -3,4 +3,6 @@ | |||
set -e | |||
source "$(dirname $0)/_jenkins_setup.sh" | |||
|
|||
"$(npm bin)/grunt" build --release; |
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.
Is this ok to do? Do we care that we're not running tests against the SNAPSHOT build?
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 don't know, doesn't look bad to me. Maybe @LeeDr has input?
jenkins, test this |
Does anyone know, or is it written down anywhere, what is responsible for building things in |
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.
For consistency, I'd consider adding the testUiReleaseServer
task as a runnable to the package.json as well.
Code looks good to me. Not sure why the last build was aborted though... |
jenkins, test this |
@@ -6,6 +6,8 @@ module.exports = function (grunt) { | |||
const root = p => resolve(__dirname, '../../', p); | |||
const binScript = /^win/.test(platform) ? '.\\bin\\kibana.bat' : './bin/kibana'; | |||
const buildScript = /^win/.test(platform) ? '.\\build\\kibana\\bin\\kibana.bat' : './build/kibana/bin/kibana'; | |||
const pkgVersion = grunt.config.get('pkg.version'); | |||
const releaseBinScript = `./build/kibana-${pkgVersion}-linux-x86_64/bin/kibana`; |
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.
can we make this dynamic? a combination of os.platform() and os.arch() should get us there
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.
- make dynamic; a combination of os.platform() and os.arch()
Leaving this undone, we will want to revisit and make sure the tests can run on more than just linux-x86_64, but for now this it.
@cjcenizal there's no difference between a snapshot and release build other than the name. Some portions of the code will reference the name (like advanced settings stored at .kibana/config/{version} and the status api). The |
@thomasneirynck re: adding the I think we are moving away from having |
@archanid Can you backport this? At least to 5.3, though it may be worth putting all the way back to 5.0 if it backports cleanly. |
Come to think of it, it'll probably be difficult to backport beyond the 5.4 branch since previous versions do not have the new functional test runner. We should at least backport to 5.x and 5.4 though. |
* build before running selenium * change directory into built snapshot * WIP: what is in the build dir * WIP: skip unit tests * run selenium on build * [functional tests on build] Derive version * [functional tests on build] Missing folder name prefix * WIP: debug on ubuntu VM * WIP: double kibana stabilize timeout * WIP: remove ubuntu VM debug change * WIP: add logs for docParams * WIP: add logs for es * build release version without -SNAPSHOT- * [functional test release] Cleanup
* build before running selenium * change directory into built snapshot * WIP: what is in the build dir * WIP: skip unit tests * run selenium on build * [functional tests on build] Derive version * [functional tests on build] Missing folder name prefix * WIP: debug on ubuntu VM * WIP: double kibana stabilize timeout * WIP: remove ubuntu VM debug change * WIP: add logs for docParams * WIP: add logs for es * build release version without -SNAPSHOT- * [functional test release] Cleanup
* build before running selenium * change directory into built snapshot * WIP: what is in the build dir * WIP: skip unit tests * run selenium on build * [functional tests on build] Derive version * [functional tests on build] Missing folder name prefix * WIP: debug on ubuntu VM * WIP: double kibana stabilize timeout * WIP: remove ubuntu VM debug change * WIP: add logs for docParams * WIP: add logs for es * build release version without -SNAPSHOT- * [functional test release] Cleanup
Fixes #7321
This PR updates the jenkins selenium task that runs on jenkins for each build. Instead of starting up the kibana script in bin/kibana, now we will run the
build --release
task and start up kibana frombuild/kibana-${version}-linux-x86_64/bin/kibana
. Then we run our functional tests on that.It has passed ✅ green on jenkins thrice and is ready for your review!