Fix integ tests for windows platform #3811
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Zelin Hao <[email protected]>
zelinh
requested review from
dblock,
peterzhuamazon,
bbarani,
gaiksaya,
rishabh6788,
jordarlu,
prudhvigodithi,
Divyaasm and
tianleh
as code owners
Codecov Report
@@ Coverage Diff @@
## main #3811 +/- ##
==========================================
+ Coverage 91.54% 91.61% +0.06%
==========================================
Files 182 182
Lines 5420 5438 +18
==========================================
+ Hits 4962 4982 +20
+ Misses 458 456 -2
|
gaiksaya
reviewed
Jul 25, 2023
src/system/process.py
Outdated
Comment on lines
70
to
71
| except Exception: | ||
| pass |
There was a problem hiding this comment.
In case of exception, do we want to retry/ print / rethrow the exception? Not sure what we are trying to do in this entire try except block.
There was a problem hiding this comment.
Yeah make sense. Let me add some more for exception handle.
src/system/process.py
Outdated
Comment on lines
80
to
86
| for proc in psutil.process_iter(): | ||
| try: | ||
| for item in proc.open_files(): | ||
| if self.stderr.name == item.path: | ||
| logging.warning(f"stderr {item} is being used by process with this detail {proc}") | ||
| except Exception: | ||
| pass |
Signed-off-by: Zelin Hao <[email protected]>
peterzhuamazon
requested changes
Jul 25, 2023
Signed-off-by: Zelin Hao <[email protected]>
peterzhuamazon
approved these changes
Jul 26, 2023
gaiksaya
reviewed
Jul 26, 2023
gaiksaya
reviewed
Jul 26, 2023
gaiksaya
approved these changes
Jul 26, 2023
prudhvigodithi
pushed a commit
to prudhvigodithi/opensearch-build
that referenced
this pull request
Jul 31, 2023
Signed-off-by: Zelin Hao <[email protected]> Signed-off-by: Prudhvi Godithi <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixing the current error of unable to remove temp files when terminating the process and also adding
UTF-8encoding when executing the process for results record purpose.According to the module document for
NamedTemporaryFile, the temp file is open when we creating it and it may be supported to re-open it again by its name on Unix but not especially on windows platform.Also during my testing, the file was still open by python process even after we closed it by the file name.
item is popenfile(path='C:\\Users\\ADMINI~1\\AppData\\Local\\Temp\\2\\tmp2mlahvrq', fd=-1) and proc is psutil.Process(pid=6168, name='python.exe', status='running', started='23:49:13'). Adding the for loop to check if it's closed before we remove it from the system will avoidPermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\ADMINI~1\\AppData\\Local\\Temp\\2\\tmpix2x74us'which caused the issue before.When the process completed, our current
test-recorderis not able to record thestdout.txtfor the dashboards on windows because it usescp1252for encoding by default and we would getUnicodeDecodeError: 'charmap' codec can't decode byte 0x90 in position 1357: character maps to <undefined>. I am addingutf-8encoding to the subprocess execution to resolve this.Sample running tests:
Running on dashboards:
./test.sh integ-test ./manifests/2.8.0/opensearch-dashboards-2.8.0-test.yml -p opensearch=https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.8.0/7935/windows/x64/zip opensearch-dashboards=https://ci.opensearch.org/ci/dbc/distribution-build-opensearch-dashboards/2.8.0/6182/windows/x64/zip --test-run-id 1234 --component alertingDashboardsRunning for OpenSearch:
./test.sh integ-test ./manifests/2.8.0/opensearch-2.8.0-test.yml -p opensearch=https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.8.0/7935/windows/x64/zip --component alerting --test-run-id 123I have also run integ tests on our linux docker and the back compatibility of the changes is passing.
[opensearch@a49844d86db9 opensearch-build]$ ./test.sh integ-test ./manifests/2.8.0/opensearch-dashboards-2.8.0-test.yml -p opensearch=https://ci.opensearch.org/ci/dbc/distribution-build-opensearch/2.8.0/7935/linux/x64/tar opensearch-dashboards=https://ci.opensearch.org/ci/dbc/distribution-build-opensearch-dashboards/2.8.0/6182/linux/x64/tar --test-run-id 1236 --component mlCommonsDashboardsIssues Resolved
#3196
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.