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

tests: Catch errors during tests setup #225

Merged
merged 2 commits into from
Aug 6, 2020

Conversation

stanislavlevin
Copy link
Contributor

Fixes: #224

Copy link
Contributor

@simo5 simo5 left a comment

Choose a reason for hiding this comment

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

I do not find is_not_setup intuitive, but the fix is technically right, I provided two alternatives that will belnd better with the existing code, let me know what you think.

@@ -801,7 +801,10 @@ def test_mech_name(testdir, testenv, logfile):
processes = dict()
logfile = open(os.path.join(testdir, 'tests.log'), 'w')
errs = 0
is_not_setup = True
Copy link
Contributor

Choose a reason for hiding this comment

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

By "is_not_setup" you mean the script "is not in the setup phase" ?

It's a bit confusing to work by negatives, I would rather have a guard named setup_phase = True, and at the end you set setup_phase = False

However an even simpler way to deal with this i to set errs to -1 and then clear it with a comment that says why before starting tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! changed to 'errs=-1'. Thank you.

@@ -15,7 +15,7 @@ elif [ -f /etc/fedora-release ]; then
dnf -y install $COMPILER python3-{gssapi,requests{,-gssapi},flake8} \
krb5-{server,workstation,pkinit} curl libfaketime \
{httpd,krb5,openssl,gssntlmssp}-devel {socket,nss}_wrapper \
autoconf automake libtool which bison make python3 \
autoconf automake libtool which bison make python3 python3-devel \
Copy link
Contributor

Choose a reason for hiding this comment

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

looks ok to me but I'd defer to @frozencemetery on this

Copy link
Member

Choose a reason for hiding this comment

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

Seems fine :)

Fixes: gssapi#224
Signed-off-by: Stanislav Levin <slev@altlinux.org>
python3-devel is required to build python-gssapi within
virtualenv.

Signed-off-by: Stanislav Levin <slev@altlinux.org>
@@ -15,7 +15,7 @@ elif [ -f /etc/fedora-release ]; then
dnf -y install $COMPILER python3-{gssapi,requests{,-gssapi},flake8} \
krb5-{server,workstation,pkinit} curl libfaketime \
{httpd,krb5,openssl,gssntlmssp}-devel {socket,nss}_wrapper \
autoconf automake libtool which bison make python3 \
autoconf automake libtool which bison make python3 python3-devel \
Copy link
Member

Choose a reason for hiding this comment

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

Seems fine :)

@simo5 simo5 merged commit 70c90bf into gssapi:master Aug 6, 2020
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.

self-tests false positive results
3 participants