Skip to content
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

FOGL-7699 Add a pytest fixture in conftest.py which collects support bundle of fledge #1059

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

Mohit04tomar
Copy link
Contributor

No description provided.

@Mohit04tomar
Copy link
Contributor Author

I have ran this Jenkins Job with changes --> http://jenkins.dianomic.com:8080/view/PI%20stuff/job/fledge_packages_pi_web_api/517/

@Mohit04tomar
Copy link
Contributor Author

This is the latest job build of this PR --> http://jenkins.dianomic.com:8080/view/PI%20stuff/job/fledge_packages_pi_web_api/519/

Comment on lines 677 to 682
conn = http.client.HTTPConnection("{}".format(url))
conn.request("POST", "/fledge/support")
r = conn.getresponse()
assert 200 == r.status
r = r.read().decode()
jdoc = json.loads(r)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

def post_request from utils.py can be used instead of this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for that I have to import utils into conftest file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is good, we should use utils def

r = r.read().decode()
jdoc = json.loads(r)
assert jdoc["bundle created"]
subprocess.run(["mkdir -p {0}/support/ && cp -r {1} {0}/support/.".format(PROJECT_ROOT, jdoc["bundle created"])], shell=True, check=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of package based system tests, what will be the PROJECT_ROOT? Are support bundles stored in PROJECT_ROOT path?
What is the value for jdoc["bundle created"] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PROJECT_ROOT is the directory that contains the tests to be executed.
Support Bundle store in PROJECT_ROOT/support/ directory and jdoc["bundle created"] is FLEDGE_ROOT/data/support/support_bundle.tar.gz

@pytest.fixture(scope="function", autouse=True)
def collect_support_bundle(request):
def _collect_support_bundle():
PROJECT_ROOT = subprocess.getoutput("git rev-parse --show-toplevel")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the old way, We can fix it later at all places with other PR

Comment on lines 677 to 682
conn = http.client.HTTPConnection("{}".format(url))
conn.request("POST", "/fledge/support")
r = conn.getresponse()
assert 200 == r.status
r = r.read().decode()
jdoc = json.loads(r)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is good, we should use utils def

Comment on lines 689 to 692
if OS_NAME == 'Ubuntu':
LOG_FILE = "/var/log/syslog"
elif OS_NAME == 'CentOS Stream':
LOG_FILE = "/var/log/messages"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have PKG_MGR to identify platform

LOG_FILE = "/var/log/syslog"
elif OS_NAME == 'CentOS Stream':
LOG_FILE = "/var/log/messages"
subprocess.run(["mkdir -p {0}/support/ && cp {1} {0}/support/syslog_{2}".format(PROJECT_ROOT, LOG_FILE, time.strftime("%Y_%m_%d_%H_%M_%S"))], shell=True, check=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's put {0}/support/syslog_{2} in save_to to copy_to variable to keep this statement short

subprocess.run(["mkdir -p {0}/support/ && cp {1} {0}/support/syslog_{2}".format(PROJECT_ROOT, LOG_FILE, time.strftime("%Y_%m_%d_%H_%M_%S"))], shell=True, check=True)

url = request.config.getoption("--fledge-url")
request.addfinalizer(_collect_support_bundle)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need addfinalizer() function to execute this fixture at end of each test functions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need only at end of test module not each function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requirement arises due to the common practice in system tests, where each test function resets the fledge before testing starts. This reset purges older data, regardless of whether the test passes or not. Consequently, the status of the fledge at that specific time remains unknown to us.

LOG_FILE = "/var/log/messages"
subprocess.run(["mkdir -p {0}/support/ && cp {1} {0}/support/syslog_{2}".format(PROJECT_ROOT, LOG_FILE, time.strftime("%Y_%m_%d_%H_%M_%S"))], shell=True, check=True)

url = request.config.getoption("--fledge-url")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use fledge_url def for that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried fledge_url def but that was not working, So I have used this request.config.getoption("--fledge-url")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to use another fixture as arg collect_support_bundle(request, fledge_url):

@Mohit04tomar
Copy link
Contributor Author

The Jenkins job run on latest commit --> http://jenkins.dianomic.com:8080/view/PI%20stuff/job/fledge_packages_pi_web_api/521/

tests/system/python/conftest.py Outdated Show resolved Hide resolved
subprocess.run(["mkdir -p {0}/support/ && cp {1} {0}/support/syslog_{2}".format(PROJECT_ROOT, LOG_FILE, time.strftime("%Y_%m_%d_%H_%M_%S"))], shell=True, check=True)

url = request.config.getoption("--fledge-url")
request.addfinalizer(_collect_support_bundle)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need only at end of test module not each function.

LOG_FILE = "/var/log/messages"
subprocess.run(["mkdir -p {0}/support/ && cp {1} {0}/support/syslog_{2}".format(PROJECT_ROOT, LOG_FILE, time.strftime("%Y_%m_%d_%H_%M_%S"))], shell=True, check=True)

url = request.config.getoption("--fledge-url")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to use another fixture as arg collect_support_bundle(request, fledge_url):

Comment on lines 681 to 682
copy_to = "mkdir -p {0}/support/ && cp -r {1} {0}/support".format(PROJECT_ROOT, jdoc['bundle created'])
subprocess.run(["{}/.".format(copy_to)], shell=True, check=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

   copy_to_cmd = "mkdir -p {0}/support/ && cp -r {1} {0}/support/.".format(PROJECT_ROOT, jdoc['bundle created'])
   subprocess.run([copy_to_cmd], shell=True, check=True)

Comment on lines 690 to 691
copy_to = "mkdir -p {0}/support/ && cp -r {1} {0}/support".format(PROJECT_ROOT, LOG_FILE)
subprocess.run(["{0}/syslog_{1}".format(copy_to, time.strftime("%Y_%m_%d_%H_%M_%S"))], shell=True, check=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

 ts = time.strftime("%Y_%m_%d_%H_%M_%S")
 copy_to_cmd = "mkdir -p {0}/support/logs/ && cp {1} {0}/support/logs/syslog_{1}".format(PROJECT_ROOT, log_file, ts)
 subprocess.run([copy_to_cmd], shell=True, check=True)

tests/system/python/conftest.py Outdated Show resolved Hide resolved
tests/system/python/conftest.py Show resolved Hide resolved
Signed-off-by: Mohit Singh Tomar <[email protected]>
@Mohit04tomar Mohit04tomar requested a review from praveen-garg May 5, 2023 10:27
@@ -668,6 +670,29 @@ def _disable_sch(fledge_url, sch_name):
return _disable_sch


@pytest.fixture(scope="function", autouse=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You typically use autouse fixtures when you want to use them for setup/teardown only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I have used an autouse fixture with a function-limited scope. This ensures that it is executed at the end of every test function/case in any system test, allowing it to collect a support bundle before executing next test function/case.

@pytest.fixture(scope="function", autouse=True)
def collect_support_bundle(request, fledge_url):
def _collect_support_bundle():
PROJECT_ROOT = Path(__file__).absolute().parent.parent.parent.parent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong practice. We should avoid this and maybe use Pathlib which is a native Python library for handling files and paths and also provides a making code more readable and maintainable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are currently utilizing the Pathlib library, given that the Path() function is associated with this library. As an alternative approach, we could use the "git rev-parse --show-toplevel" command to pinpoint the precise path of the PROJECT_ROOT. However, I will explore more optimal methods to determine this path.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try this code snippet to get PROJECT_ROOT

import pathlib
PROJECT_NAME="tests"
PROJECT_ROOT = str(pathlib.Path().resolve()).split(PROJECT_NAME)[0]
print(PROJECT_ROOT)

Comment on lines 686 to 690
log_file = "/var/log/syslog"
if pytest.PKG_MGR == 'yum':
log_file = "/var/log/messages"
ts = time.strftime("%Y_%m_%d_%H_%M_%S")
copy_to_cmd = "mkdir -p {0}/support/logs/ && cp {1} {0}/support/logs/syslog_{2}".format(PROJECT_ROOT, log_file, ts)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IN case of exception we should only get the Fledge related logs. Not the full dump from path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I'll update the code to only get logs related to Fledge.

Comment on lines +817 to +823
print("Copying syslog")
log_file = "/var/log/syslog"
if pytest.PKG_MGR == 'yum':
log_file = "/var/log/messages"
ts = time.strftime("%Y_%m_%d_%H_%M_%S")
copy_to_cmd = f"mkdir -p {PROJECT_ROOT}/support/logs/ && grep -i 'fledge' {log_file} > {PROJECT_ROOT}/support/logs/syslog_{ts}"
subprocess.run([copy_to_cmd], shell=True, check=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixture should have flexibility to collect support bundle even in success case as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants