-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Use mage target instead of nosetests on Windows jenkins CI #16141
Use mage target instead of nosetests on Windows jenkins CI #16141
Conversation
Mage targets manage their own virtual environments.
@@ -65,5 +65,5 @@ exec { go test -race -c -cover -covermode=atomic -coverpkg $packages } "go test | |||
|
|||
if (Test-Path "tests\system") { | |||
Set-Location -Path tests\system | |||
exec { nosetests --with-timer --with-xunit --xunit-file=../../build/TEST-system.xml } "System test FAILURE" | |||
exec { mage pythonUnitTest } "System test FAILURE" |
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.
Command executed by pythonUnitTest
is:
exec: <...>/github.com/elastic/beats/build/ve/linux/bin/nosetests --process-timeout=90 --with-timer -v --with-xunit --xunit-file=build/TEST-python-unit.xml <...>
This is progressing, only serious pending problem is that x-pack builds for Metricbeat and auditbeat are failing when creating the virtual environment, with the following error:
For example here: https://beats-ci.elastic.co/job/elastic+beats+pull-request+multijob-windows/6197/beat=x-pack%2Fmetricbeat,label=windows/console It is interesting because it doesn't happen in other builds that run in principle the same command, I am trying to reproduce it locally. There are also a couple of errors that I am considering ignoring because they don't affect the builds: I see the following error, but then tests are run correctly, it also seems to happen with other calls to this
For example here: https://beats-ci.elastic.co/job/elastic+beats+pull-request+multijob-windows/6197/beat=filebeat,label=windows/console Sometimes Python 3 installation "fails" with the following error, but then everything works ok:
For example here: https://beats-ci.elastic.co/job/elastic+beats+pull-request+multijob-windows/6197/beat=auditbeat,label=windows/console // cc @andrewkroh @mikemadden42 @kuisathaverat any idea about any of these errors? |
I really hate PowerShell error reporting, I guess it continues because we do not have defined
I think that the real failure is that can not change the PATH variable
The only thing that comes to my mind is that can be the length of the path, I will try to replicate it on a bat file to see if the error is reported. |
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 progressing, only serious pending problem is that x-pack builds for Metricbeat and auditbeat are failing when creating the virtual environment, with the following error:
...
11:37:24 \label\windows\src\github.com\elastic\beats\build\ve\windows\Scripts\python.exe', '-Im', 'ensurepip',
11:37:24 '--upgrade', '--default-pip']' returned non-zero exit status 1.
Try setting PTYHON_ENV to a tmp directory that is shorter in length. That paths to the VE might be too long.
I saw a similar error when updating the Vagrantfile in #15831. When I moved the PYTHON_ENV
location off of the virtualbox mount point it started working. I don't think this is same issue since its not a network mount location, but moving the PYTHON_ENV directory to a tmp location might help.
Regarding the exec
in powershell, I concur on the dislike of PowerShell. You need a lot of experience to get it right. My strong recommendation is to ditch this script. Bootstrap Go with gvm then immediately invoke something like go run jenkins_windows.go
that does all of the other setup that the script used to do.
@@ -528,7 +528,7 @@ def test_utf8(self): | |||
lambda: self.output_has(lines=1), max_timeout=10) | |||
|
|||
# Append utf-8 chars to check if it keeps reading | |||
with codecs.open(testfile, "a") as f: | |||
with codecs.open(testfile, "a", "utf-8") as f: |
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.
utf_8
is the name of the codec in Python. utf-8
is an alias name. I think I used utf_8
in other places. So to prevent having a mix of "utf-8" and "utf_8" can you change this to utf_8
.
I have changed the I agree with rewriting this script in go (or mage), but wdyt about doing it in a follow up? Thanks @kuisathaverat and @andrewkroh! |
Awesome! Yeah, let's merge this. |
…6141) Replaces use of nosetests with mage wrapper in script for Windows CI workers. Mage manages their own python virtual environments, this is preferred to directly call python commands. Installs python 3 using chocolatey, as it is still not available in Windows CI workers. future was failing to install in Windows, but we don't actually need it anymore, so it is removed.
What does this PR do?
Replaces use of nosetests with mage wrapper in script for Windows CI workers.
Mage manages their own python virtual environments, this is preferred to directly
call python commands.
Installs python 3 using chocolatey, as it is still not available in windows CI workers.
future
was failing to install in Windows, but we don't actually need it anymore,so it is removed.
Why is it important?
To fix Windows builds in Python 3 branch (#14798).