Python configuration for tests #2793
Conversation
narrieta
requested review from
ZhidongPeng,
nagworld9 and
maddieford
as code owners
| @@ -277,54 +277,74 @@ def _setup_node(self) -> None: | |||
| log.info("Resource Group: %s", self.context.vm.resource_group) | |||
| log.info("") | |||
|
|
|||
| self.context.ssh_client.run_command("mkdir -p ~/bin/tests_e2e/tests; touch ~/bin/agent-env") | |||
There was a problem hiding this comment.
Now we prepare a tarball on the orchestrator machine that is already structured in the way it needs to be setup on the test VMs, instead of using SSH to create/copy these files. This should make easier to include additional files that need to be copied to the test VMs.
| @@ -447,15 +469,15 @@ def _execute_test_suite(self, suite: TestSuiteInfo) -> bool: | |||
| test_start_time: datetime.datetime = datetime.datetime.now() | |||
|
|
|||
| log.info("******** Executing %s", test_name) | |||
| self.context.lisa_log.info("******** Executing %s", test_full_name) | |||
| self.context.lisa_log.info("Executing test %s", test_full_name) | |||
There was a problem hiding this comment.
Minor formatting change. We used to output "********" to the LISA log to make those messages stand out. My previous PR created a log for each environment, so now the LISA log is not too crowded and this marker is not needed anymore.
| @@ -528,7 +550,7 @@ def _check_agent_log(self) -> bool: | |||
| self.context.lisa_log.info("Checking agent log on the test node") | |||
| log.info("Checking agent log on the test node") | |||
|
|
|||
| output = self.context.ssh_client.run_command("check-agent-log.py -j") | |||
| output = self.context.ssh_client.run_command("$(get-agent-python) ~/bin/check-agent-log.py -j") | |||
There was a problem hiding this comment.
get_distro() in version.py is not returning the expected values in some distros when using Pypy. I need to follow up that, and in the meanwhile I am using the system's python to execute this script.
| @@ -551,21 +573,17 @@ def _check_agent_log(self) -> bool: | |||
| log.info("There are no errors in the agent log") | |||
| return True | |||
|
|
|||
| log_path: Path = self.context.log_path/f"CheckAgentLog-{self.context.environment_name}.log" | |||
There was a problem hiding this comment.
My previous PR create a log for each environment, so now we do not need to create this extra CheckAgentLog-*.log, we can simply log those messages to the environment log.
| # | ||
| set -euo pipefail | ||
|
|
||
| # python3 is available on most distros |
There was a problem hiding this comment.
Somehow git captures this as a rename + changes. I actually removed find-python and created a totally unrelated script, get-agent-modules-path
| @@ -1,5 +1,3 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
This script runs using whatever Python is installed on the test VM, and that is not necessarily Python 3.
| """ | ||
| Renames the given log to prefix it with "_". | ||
| Adds a message to indicate the log contains errors. |
There was a problem hiding this comment.
Marking failed logs by renaming them makes developer runs a little harder, since one may be watching the progress in a test log while the file is renamed. Instead, we add the message below and do not change the name.
| # Returns the PYTHONPATH on which the azurelinuxagent and associated modules are located. | ||
| # | ||
| # To do this, the script tries to find the python command used to start the agent and then | ||
| # returns the value of site.getsitepackages(). |
There was a problem hiding this comment.
Need to change the comment. Now this script returns agent python executable path
| # ln -s "$python" ~/bin/python3 | ||
| # fi | ||
| # fi | ||
| #fi |
There was a problem hiding this comment.
This is commented, are we still figuring out for flatcar?
Codecov Report
@@ Coverage Diff @@
## develop #2793 +/- ##
========================================
Coverage 72.01% 72.01%
========================================
Files 104 104
Lines 15870 15870
Branches 2273 2273
========================================
Hits 11429 11429
Misses 3916 3916
Partials 525 525 see 2 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| @@ -86,18 +86,27 @@ sudo find "$LOGS_DIRECTORY" -exec chown "$USER" {} \; | |||
| # | |||
| # Move the relevant logs to the staging directory | |||
| # | |||
| # Move the logs for failed tests to a temporary location | |||
There was a problem hiding this comment.
Why do we move failed tests to a temporary location here?
There was a problem hiding this comment.
we move them back on line 105
the temp location makes it easy to separate them from other logs
This PR changes the Python configuration for the end to end tests to an explicit approach: instead of having a default Python, tests need to specify the Python to use (the system's Python or Pypy).
The PR also includes some minor cleanup fixes. I'll point them out in review comments.