tests: ported authselect tests from downstream by danlavu · Pull Request #349 · authselect/authselect · GitHub
Skip to content

tests: ported authselect tests from downstream#349

Merged
pbrezina merged 1 commit into
authselect:masterfrom
danlavu:tests-authselect
Apr 4, 2024
Merged

tests: ported authselect tests from downstream#349
pbrezina merged 1 commit into
authselect:masterfrom
danlavu:tests-authselect

Conversation

@danlavu

@danlavu danlavu commented Aug 24, 2023 •
edited
Loading

Copy link
Copy Markdown
  • adding authselect tests
  • adding test files copied from sssd
    • conftest.py
    • data/readme.md
    • pytest.ini
    • pyproject.toml
    • requirements.txt
    • setup.cfg

@danlavu danlavu mentioned this pull request Aug 24, 2023
Closed
@danlavu

danlavu commented Aug 24, 2023

Copy link
Copy Markdown
Author

@pbrezina

pbrezina commented Sep 22, 2023

Copy link
Copy Markdown
Member

RHEL8 and RHEL9 are stuck with 1.2.x version, how do we want to handle this? Keep old tests for RHEL8 and 9 and start using mh tests for RHEL10? Or backport it also to 1.2.x?

@pbrezina

pbrezina commented Sep 22, 2023

Copy link
Copy Markdown
Member

Is this ready for review? I think you said that you are still working on it.

@danlavu

danlavu commented Sep 22, 2023

Copy link
Copy Markdown
Author

These tests should work for 1.2, I thought we discussed major releases we will use git branches, and minor releases, use pytest markers.

I need to finish the PAM PR before this can be merged.

@pbrezina

pbrezina commented Sep 22, 2023

Copy link
Copy Markdown
Member

These tests should work for 1.2, I thought we discussed major releases we will use git branches, and minor releases, use pytest markers.

If these will work for 1.2 then we can backport it to 1.2.x and run it for 8 and 9 from this branch. RHEL10 can use master.

I need to finish the PAM PR before this can be merged.

Thanks, let me know when it is ready for review.

@pbrezina pbrezina marked this pull request as draft September 22, 2023 18:14
@danlavu danlavu marked this pull request as ready for review November 3, 2023 12:23
@danlavu danlavu force-pushed the tests-authselect branch 2 times, most recently from 1fc6ec9 to c76673c Compare November 28, 2023 17:22
@danlavu

danlavu commented Nov 28, 2023

Copy link
Copy Markdown
Author

@pbrezina This is ready for review.

Also, I do not have developer access to authselect, so I'm unable to assign this issue to myself.

@thalman thalman self-assigned this Nov 30, 2023
@pbrezina

pbrezina commented Dec 4, 2023

Copy link
Copy Markdown
Member

Hi, I invited you to the organization.

I am not sure if I will be able to review this before the Christmas break (I'll try). Can you please also make the tests run during PR CI? See SSSD's ci.yaml for reference.

pbrezina
pbrezina requested changes Jan 9, 2024
View reviewed changes

@pbrezina pbrezina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi Dan,
please, include mhc.yaml, polarion.yaml .

All tests here are for authselect, so test_authselect.py and test_authselect prefix does not make sense. It would be better to organize the test differently eg by profile.

Included test may be organized as:

  • test_sssd
  • test_minimal
  • test_distribution (eg test_authselect__when_removed_the_uninstall_is_clean)

Tests "enable_and_disable" are not sufficient. Presence of specific pam module is irrelevant, what matters is functionality. You must test that the functionality is working correctly.

test_authselect__when_removed_the_uninstall_is_clean - the problem is not RHEL vs Fedora but behavior change between authselect version. This test belongs only to 1.2.x branch.

@danlavu

danlavu commented Jan 15, 2024

Copy link
Copy Markdown
Author

@pbrezina ack, changing a lot of things and removing the non-functional tests, and making the placeholder/sanity into functional before the next review.

@danlavu danlavu marked this pull request as draft January 17, 2024 02:01
@danlavu danlavu force-pushed the tests-authselect branch 4 times, most recently from 4a5218b to 39356d4 Compare February 18, 2024 19:34
@danlavu

danlavu commented Feb 18, 2024

Copy link
Copy Markdown
Author

@pbrezina This is ready for review now. It depends on the following PRs.

SSSD/sssd-test-framework#86
SSSD/sssd-test-framework#85
SSSD/sssd-test-framework#72

@danlavu danlavu requested review from pbrezina and thalman February 19, 2024 14:38
@danlavu danlavu marked this pull request as ready for review February 19, 2024 14:38
pbrezina
pbrezina requested changes Feb 20, 2024
View reviewed changes

@pbrezina pbrezina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, this is much better and I think we can go with this for now.

In long term, however, I would like to rework it to be less sssd-like and more focused on authselect. We can not test winbind or nis stuff with the sssd framework. Lots of features are shared between profiles so we could use topology parametrization here, with the topology representing a profile. We are not that much interested in the sssd providers since we rely on the posix interface.

But its good enough for the moment. Thank you.

Comment thread src/tests/system/polarion.yaml
Comment thread src/tests/system/tests/test_minimal.py Outdated
@danlavu danlavu requested a review from pbrezina March 21, 2024 17:09
pbrezina
pbrezina requested changes Apr 3, 2024
View reviewed changes

@pbrezina pbrezina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi Dan, see the inline comment. Also please, rebase it properly without the merge commit and we can push. Thank you.

Comment thread src/tests/system/pytest.ini Outdated
@danlavu danlavu force-pushed the tests-authselect branch 2 times, most recently from 911f7b5 to 405c7cd Compare April 3, 2024 13:31
@danlavu danlavu force-pushed the tests-authselect branch from 405c7cd to a621f77 Compare April 3, 2024 13:34
@danlavu

danlavu commented Apr 3, 2024

Copy link
Copy Markdown
Author

@pbrezina pbrezina merged commit b168021 into authselect:master Apr 4, 2024
@danlavu danlavu deleted the tests-authselect branch April 17, 2024 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pbrezina pbrezina pbrezina approved these changes
@thalman thalman Awaiting requested review from thalman

Assignees

@thalman thalman

@pbrezina pbrezina

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@danlavu @pbrezina @thalman