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

test: add test script #169

Closed
wants to merge 65 commits into from

Conversation

PhilipTamb
Copy link
Contributor

This pull request wants to add the possibility to test a specific Zenoh-Flow configuration using a script

@PhilipTamb PhilipTamb marked this pull request as ready for review October 10, 2023 15:54
@J-Loudet J-Loudet marked this pull request as draft October 11, 2023 22:00
Copy link
Contributor

@gabrik gabrik left a comment

Choose a reason for hiding this comment

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

When printing use the formatter with print(f'Some thing: {variable_name}')
https://docs.python.org/3/tutorial/inputoutput.html#tut-f-strings

manual-tests/README.md Outdated Show resolved Hide resolved
manual-tests/automated-test.py Outdated Show resolved Hide resolved
manual-tests/automated-test.py Outdated Show resolved Hide resolved
manual-tests/automated-test.py Outdated Show resolved Hide resolved
Copy link
Contributor

@J-Loudet J-Loudet left a comment

Choose a reason for hiding this comment

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

Solid basis. Needs adjustments.

manual-tests/automated-test.py Outdated Show resolved Hide resolved
Comment on lines 108 to 119
if (
args.zenohd == "/.local/bin/zenohd"
or args.zfctl == "/.config/zenoh-flow/zfctl.json"
):
print(home_path)
zf_conf.zenohd_path = home_path + str(args.zenohd)
zf_conf.zfctl_path = home_path + str(args.zfctl)
zf_conf.zf_plugin_path = home_path + str(args.plugin)
else:
zf_conf.zenohd_path = args.zenohd
zf_conf.zfctl_path = args.zfctl
zf_conf.zf_plugin_path = args.plugin
Copy link
Contributor

Choose a reason for hiding this comment

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

It is better to put the correct path in the default options. If users run the "help" option and see "/.config/…" they can assume that something is wrong.

There is the function: os.path.expanduser().

What I suggest you do: provide a default option with ~ in the path & call expanduser in all cases (be it the default option or provided by the user).

)
print(f'show list of Zenoh-Flow instances: \n {zf_command["instances_cmd"]}')

print("[Info] Looking for Zenohd process...")
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that you are not just looking for zenohd processes, I would rather write:

Suggested change
print("[Info] Looking for Zenohd process...")
print("[Info] Killing extra `zenohd` processes...")

Comment on lines 230 to 232
pid = process.pid
print(f"[Info] kill process: {pid}")
os.kill(pid, signal.SIGKILL)
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't this be simplified like so?

Suggested change
pid = process.pid
print(f"[Info] kill process: {pid}")
os.kill(pid, signal.SIGKILL)
print(f"[Info] kill process: {process.pid}")
process.kill()

Comment on lines 271 to 289
print(f'[Info] Launching list instances: {zf_command["instances_cmd"]}')
launch_instances = os.popen(zf_command["instances_cmd"])
count = 0
instances_string = launch_instances.read().split()
for i in range(instances_string.__len__()):
if instances_string[i][0] == "+":
count = count + 1
if count <= 2:
print("[Error] test data flow: FAILED.")
exit()
else:
print("[Info] test data flow instance: SUCCESS.")
instance["uuid_instances"] = instances_string[23]
instance["flow"] = instances_string[25]
instance["operators"] = instances_string[27]
instance["sinks"] = instances_string[29]
instance["sources"] = instances_string[31]
instance["connectors"] = instances_string[33]
instance["links"] = instances_string[35]
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove these lines, they are too specific and will quickly return false negatives.
They are referred to as "magic numbers" and are considered bad practice.

Comment on lines 300 to 317
if str(tuple(data["sources"]).__len__()) == instance["sources"]:
print(
f'[Info] test active Source nodes: SUCCESS, the active Source nodes [{str(tuple(data["sources"]).__len__())}] are equal to declared Source nodes [{instance["sources"]}]'
)
if str(tuple(data["sinks"]).__len__()) == instance["sinks"]:
print(
f'[Info] test active Sink nodes: SUCCESS, the active Sink nodes [{str(tuple(data["sinks"]).__len__())}] are equal to those declared Sinks nodes [{instance["sinks"]}]'
)
if str(tuple(data["operators"]).__len__()) == instance["operators"]:
print(
f'[Info] test active Operator nodes: SUCCESS, the active Operator nodes [{str(tuple(data["operators"]).__len__())}] are equal declared Operator nodes [{instance["operators"]}]'
)
if (
str(tuple(data["sources"]).__len__()) != instance["sources"]
or str(tuple(data["sinks"]).__len__()) != instance["sinks"]
or str(tuple(data["operators"]).__len__()) != instance["operators"]
):
print("[Error] Test Zenoh-Flow configuration nodesSome nodes: FAILED.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Considering the previous change I asked you to do, these lines need to be removed as well.

Comment on lines 324 to 326
res = txt_res.read()
if res.split()[1] == "Test!":
print(f"\n[Info] Test Recive: SUCCESS, {res}")
Copy link
Contributor

Choose a reason for hiding this comment

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

For the test to be more reliable, you need to:

  1. open the file, count the number of lines,
  2. do the curl
  3. ensure that the file contains one more line and check that that extra line is equal to "Hello, Test!"

@PhilipTamb PhilipTamb marked this pull request as ready for review October 23, 2023 12:10
@J-Loudet J-Loudet deleted the branch eclipse-zenoh-flow:master February 7, 2024 10:19
@J-Loudet J-Loudet closed this Feb 7, 2024
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.

3 participants