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-Only] Added ldapAttributesForUserSearch in setConfig.sh #560

Merged
merged 5 commits into from
May 25, 2020

Conversation

kiranparajuli589
Copy link
Contributor

@kiranparajuli589 kiranparajuli589 commented May 25, 2020

Fixes owncloud/core#37428

Added ldapAttributesForUserSearch as uid inside setConfig.sh

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

Good stuff if CI passes.

@phil-davis
Copy link
Contributor

Why does https://github.com/owncloud-docker/compose-playground/blob/master/compose/ldap/ldap.sh have a list of settings that have underscores, like ldap_base_groups
But https://github.com/owncloud/user_ldap/blob/master/tests/acceptance/setConfig.sh has ldapBaseGroups ?

It would be nice to understand this!

@kiranparajuli589
Copy link
Contributor Author

kiranparajuli589 commented May 25, 2020

Why does https://github.com/owncloud-docker/compose-playground/blob/master/compose/ldap/ldap.sh have a list of settings that have underscores, like ldap_base_groups
But https://github.com/owncloud/user_ldap/blob/master/tests/acceptance/setConfig.sh has ldapBaseGroups ?

It would be nice to understand this!

@phil-davis Both naming convention is supported for occ commands. We can use either one!

@phil-davis phil-davis force-pushed the update-ldap-setConfig branch from 316c398 to 5bcc1ae Compare May 25, 2020 12:25
@phil-davis phil-davis changed the title [Tests-Only] Added ldap_attributes_for_user_search in setConfig.sh [Tests-Only] Added ldapAttributesForUserSearch in setConfig.sh May 25, 2020
@phil-davis
Copy link
Contributor

I pushed the changes from PRs #559 and #557 - we might as well get all that merged in a single run of CI.

I also added a commit to "Fix expected output when syncing a user that does not exist"

Hopefully we get green CI!

@kiranparajuli589
Copy link
Contributor Author

@phil-davis there was output-message change in https://github.com/owncloud/core/pull/37398/files#diff-0ada4aa7ff5e95845c939f25bf35448dL253-R277
due to which 1 acceptance test was failing.
Command output message is now also updated in acceptance tests

@kiranparajuli589 kiranparajuli589 force-pushed the update-ldap-setConfig branch from 38a6ec3 to 51e726a Compare May 25, 2020 12:45
@phil-davis
Copy link
Contributor

@kiranparajuli589 What changed when you pushed again the last commit?
I do not see what is different from the way it was.

@mrow4a
Copy link
Contributor

mrow4a commented May 25, 2020

@phil-davis
Copy link
Contributor

yes, I know. I had pushed that a while ago. Maybe I made a small mistake - I just do not see how the newly-pushed commit is any different to what I originally pushed. Anyway, it does not matter - if it passes then it is good!

@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/user_ldap/2536/48/11
Passes against core master.

https://drone.owncloud.com/owncloud/user_ldap/2536/53/11
But of course now fails against core "latest" which is 10.4.1.
I will add a commit to skip that scenario on old oC versions.

@phil-davis phil-davis merged commit 81cb1a8 into master May 25, 2020
@delete-merged-branch delete-merged-branch bot deleted the update-ldap-setConfig branch May 25, 2020 15:49
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.

user_ldap acceptance tests failing
3 participants