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

Pbench Server client library and functional tests #2941

Merged
merged 6 commits into from
Aug 5, 2022

Conversation

dbutenhof
Copy link
Member

PBENCH-810

This adds the beginnings of a simple Pbench Server python client library (with a Pbench class) and a few initial functional test modules.

The modules are written in Python using the Pytest framework to give us convenient test modularization and fixtures. We use a fixture to establish a Pbench Server connection, for example.

The functional test modules currently include a simple validation that we can connect to a server and retrieve the server's endpoints, and a test of Pbench Server user operations including register, login, logout, profile access/update, and delete.

The PBENCH_SERVER environment variable provides the server host for connection; the pbench fixture will cause failure if the environment variable isn't defined, or if connection fails.

@dbutenhof dbutenhof added Server API Of and relating to application programming interfaces to services and functions Functional Testing testing labels Jul 12, 2022
@dbutenhof dbutenhof self-assigned this Jul 12, 2022
server/requirements.txt Outdated Show resolved Hide resolved
@dbutenhof dbutenhof marked this pull request as ready for review July 13, 2022 11:35
@riya-17 riya-17 self-requested a review July 14, 2022 09:39
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

Looks great.

But, of course, I have a pile of nits for you. 🍤

exec-unittests Show resolved Hide resolved
lib/pbench/client/__init__.py Show resolved Hide resolved
lib/pbench/client/__init__.py Outdated Show resolved Hide resolved
lib/pbench/client/__init__.py Outdated Show resolved Hide resolved
lib/pbench/client/__init__.py Show resolved Hide resolved
lib/pbench/test/functional/server/test_connect.py Outdated Show resolved Hide resolved
lib/pbench/test/functional/server/test_user.py Outdated Show resolved Hide resolved
lib/pbench/test/functional/server/test_user.py Outdated Show resolved Hide resolved
@dbutenhof dbutenhof marked this pull request as draft July 15, 2022 14:41
@dbutenhof
Copy link
Member Author

Changing to draft mode for a bit: I went on autopilot while my daughter was chatting to me on the phone and pushed a commit after the functional tests ran... forgetting I'd meant to add some additional test cases. (And I didn't even blacken the code 😵‍💫 )

@dbutenhof dbutenhof marked this pull request as ready for review July 15, 2022 15:09
@dbutenhof dbutenhof requested a review from webbnh July 15, 2022 15:09
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

This is looking great, but...new code, new nits. 🥴

  • The increased use of CaseInsensitiveDict seems good...but, I think we need even more.
  • I resisted the temptation to request unit tests in my last review, but...since you've added some stuff that looks like unit testing to the functional test, I figured I would bring it up in this one. 🙂
  • A few of the exception assertions should probably use formatted strings (with an argument) -- I don't think they require raw strings.
  • I've suggested two more, easy test cases.
  • And, there are a few other, smaller things.

lib/pbench/client/__init__.py Outdated Show resolved Hide resolved
lib/pbench/client/__init__.py Outdated Show resolved Hide resolved
lib/pbench/client/__init__.py Outdated Show resolved Hide resolved
lib/pbench/test/functional/server/conftest.py Outdated Show resolved Hide resolved
lib/pbench/test/functional/server/test_connect.py Outdated Show resolved Hide resolved
lib/pbench/test/functional/server/test_connect.py Outdated Show resolved Hide resolved
lib/pbench/test/functional/server/test_user.py Outdated Show resolved Hide resolved
lib/pbench/test/functional/server/test_user.py Outdated Show resolved Hide resolved
lib/pbench/test/functional/server/test_user.py Outdated Show resolved Hide resolved
npalaska
npalaska previously approved these changes Jul 20, 2022
Copy link
Member

@npalaska npalaska left a comment

Choose a reason for hiding this comment

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

This looks really great!

Copy link
Member Author

@dbutenhof dbutenhof left a comment

Choose a reason for hiding this comment

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

The idea of adding a third major component to the test scripts is daunting (there are many semi-hidden dependencies I suspect will be ugly to resolve), and the idea of piggybacking unit testing onto either of the existing major components is deeply unsatisfying.

So I guess I'm going to dump this into the "draft" bin and let it stew while I think about how much effort it's actually worth. 😦

lib/pbench/client/__init__.py Outdated Show resolved Hide resolved
lib/pbench/test/functional/server/test_connect.py Outdated Show resolved Hide resolved
@dbutenhof dbutenhof marked this pull request as draft July 20, 2022 11:49
Copy link
Member

@riya-17 riya-17 left a comment

Choose a reason for hiding this comment

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

Looks good. I just had one query.

PBENCH-810

This adds the beginnings of a simple Pbench Server python client
library (with a `Pbench` class) and a few initial functional test
modules.

The modules are written in Python using the Pytest framework to
give us convenient test modularization and fixtures. We use a
fixture to establish a Pbench Server connection, for example.

The functional test modules currently include a simple validation
that we can connect to a server and retrieve the server's endpoints,
and a test of Pbench Server user operations including register,
login, logout, profile access/update, and delete.

The `PBENCH_SERVER` environment variable provides the server
host for connection; the `pbench` fixture will cause failure if the
environment variable isn't defined, or if connection fails.
Change requirements.txt back to `psycopg2`, and fix `exec-unittests` to
run only `test.functional.agent` (which aren't functional) in the CI
unit test pipeline, not `test.functional.server` (which now are real
functional tests).
. Split off "client" unit tests for connect and login
. Modified exec-unittests to recognize the extra "major"
@dbutenhof dbutenhof marked this pull request as ready for review August 2, 2022 20:04
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

This looks great!

Unfortunately, I found two small things...one of which leaves me with a medium-sized mystery....

exec-unittests Outdated Show resolved Hide resolved
lib/pbench/test/functional/server/test_user.py Outdated Show resolved Hide resolved
@dbutenhof dbutenhof requested a review from webbnh August 3, 2022 19:14
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

👍

@dbutenhof dbutenhof merged commit a7814d3 into distributed-system-analysis:main Aug 5, 2022
@portante portante added this to the v0.72 milestone Aug 10, 2022
@dbutenhof dbutenhof deleted the client branch March 16, 2023 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Of and relating to application programming interfaces to services and functions Functional Testing Server testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants