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: alltests/test_ldap_extra_attrs.py converted #6977

Closed
wants to merge 1 commit into from

Conversation

patriki01
Copy link
Contributor

@patriki01 patriki01 commented Oct 10, 2023

@patriki01 patriki01 force-pushed the extra_attributes branch 2 times, most recently from 45a517e to f5204be Compare October 30, 2023 09:00
@patriki01 patriki01 marked this pull request as ready for review October 30, 2023 11:31
@patriki01 patriki01 force-pushed the extra_attributes branch 2 times, most recently from 112fb2e to 9eb98d1 Compare November 2, 2023 06:57
@patriki01
Copy link
Contributor Author

I corrected my code for next round. @justin-stephenson

@justin-stephenson
Copy link
Contributor

I corrected my code for next round. @justin-stephenson

Okay let's wait for SSSD/sssd-test-framework#56 then re-run CI here before approving.

@patriki01
Copy link
Contributor Author

Blocking PR merged, rerunning this one

@patriki01
Copy link
Contributor Author

@justin-stephenson CI is green. I think that failures are unrelated. Can we proceed?

Copy link
Contributor

@justin-stephenson justin-stephenson left a comment

Choose a reason for hiding this comment

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

Ack, thank you.

Comment on lines 195 to 243
def test_ldap_extra_attrs__thread_issue(client: Client, provider: GenericProvider):
"""
:title: Thread issue can cause the application to not get any identity information
:setup:
1. Create new user "user1"
2. Edit config - "use_fully_qualified_names" to "false"
3. Start SSSD
:steps:
1. Start new thread with provided function
2. Main thread waits for worker and then sleeps
3. Worker is in loop calling getpwuid()
4. Main thread cancel worker thread (with exit_flag.set())
5. Thread join
6. Call getpwuid()
:expectedresults:
1. Thread is started
2. Success
3. Success
4. Worker is cancelled
5. Thread is joined
6. Call is not stucked
:customerscenario: True
"""

def worker():
nonlocal started, exit_flag
uid = os.geteuid()
started.set()
while not exit_flag.is_set():
pwd.getpwuid(uid)

provider.user("user1").add()
client.sssd.domain["use_fully_qualified_names"] = "false"
client.sssd.start()

started = threading.Event()
exit_flag = threading.Event()

thread = threading.Thread(target=worker)
thread.start()

started.wait()
time.sleep(3)

exit_flag.set()
thread.join()

pw = pwd.getpwuid(os.geteuid())
assert pw.pw_name, "pwd.getpwuid() call did not return anything"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this tests anything.

  • python threads are only simulated, they are not real threads
  • all code here is executed on the localhost not on the client (both threads and getpwuid)

Copy link
Member

@pbrezina pbrezina left a comment

Choose a reason for hiding this comment

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

Please, check if you can reproduce the issue if you run the python code on the client. Maybe the simulated thread is sufficient. Otherwise we will need to use a C code, which again raises a question of how do we want to handle it.

@pbrezina
Copy link
Member

For now, you can remove this test case from this PR so it can be pushed - please, resolve conflicts. You can handle it on different pull request.

@patriki01
Copy link
Contributor Author

I resolved the conflicts (Changed the name of the file) and removed last test. Now it is ready

src/tests/system/tests/test_schema.py Dismissed Show dismissed Hide dismissed
src/tests/system/tests/test_schema.py Fixed Show fixed Hide fixed
src/tests/system/tests/test_schema.py Fixed Show fixed Hide fixed
src/tests/system/tests/test_schema.py Fixed Show fixed Hide fixed
import os
import pwd
import threading
import time

Check notice

Code scanning / CodeQL

Unused import Note test

Import of 'time' is not used.
@pbrezina
Copy link
Member

pbrezina commented Dec 8, 2023

Pushed PR: #6977

  • master
    • c236081 - Tests: alltests/test_ldap_extra_attrs.py converted to system/tests/test_schema.py
  • sssd-2-9
    • 66bd91d - Tests: alltests/test_ldap_extra_attrs.py converted to system/tests/test_schema.py

@pbrezina pbrezina added Pushed and removed Accepted Ready to push Ready to push labels Dec 8, 2023
@pbrezina pbrezina closed this Dec 8, 2023
@patriki01 patriki01 deleted the extra_attributes branch December 8, 2023 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants