Skip to content

using LDAP users in UI tests - #30304

Merged
individual-it merged 3 commits into
masterfrom
ui-tests-for-ldap
Feb 3, 2018
Merged

using LDAP users in UI tests#30304
individual-it merged 3 commits into
masterfrom
ui-tests-for-ldap

Conversation

@individual-it

Copy link
Copy Markdown
Member

Description

pseudo user creation in LDAP

Motivation and Context

we would like to run tests against a LDAP backend but not going throw the problems of creating the users/groups in PHP. So when running LDAP tests (TEST_LDAP env. variable set to "true") we assume the users/groups already exist in the LDAP server

So this would be a way to run at least some acceptance tests on LDAP with very minimal changes. See changes on the LDAP side owncloud/user_ldap#173

How Has This Been Tested?

run tests in drone owncloud/user_ldap#173

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@phil-davis phil-davis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like a good/flexible thing.
Having an "LDAP" tag on some scenarios gives the implication (not true) that the scenario might be just for LDAP. I am not sure how to get around that - people just need to understand what tags are used for, and that there is an implied "empty tag" that is "run all scenarios"

@codecov

codecov Bot commented Jan 29, 2018

Copy link
Copy Markdown

Codecov Report

Merging #30304 into master will increase coverage by 0.2%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             master   #30304     +/-   ##
===========================================
+ Coverage     60.64%   60.84%   +0.2%     
  Complexity    18549    18549             
===========================================
  Files          1092     1092             
  Lines         61278    61280      +2     
===========================================
+ Hits          37160    37287    +127     
+ Misses        24118    23993    -125
Impacted Files Coverage Δ Complexity Δ
lib/private/Repair/SqliteAutoincrement.php 85.18% <0%> (-3.28%) 9% <0%> (ø)
lib/private/DB/AdapterSqlite.php 83.33% <0%> (-2.88%) 7% <0%> (ø)
lib/private/Server.php 82.84% <0%> (+0.14%) 129% <0%> (ø) ⬇️
lib/private/Files/View.php 84.41% <0%> (+0.3%) 377% <0%> (ø) ⬇️
lib/private/Repair/RepairMismatchFileCachePath.php 82.52% <0%> (+0.4%) 48% <0%> (ø) ⬇️
lib/private/DB/QueryBuilder/QueryBuilder.php 90.74% <0%> (+0.46%) 68% <0%> (ø) ⬇️
lib/private/DB/Connection.php 68.38% <0%> (+4.41%) 49% <0%> (ø) ⬇️
lib/private/DB/MDB2SchemaManager.php 85.96% <0%> (+5.26%) 17% <0%> (ø) ⬇️
...private/DB/MySqlSchemaColumnDefinitionListener.php 5.4% <0%> (+5.4%) 46% <0%> (ø) ⬇️
lib/private/DB/MDB2SchemaWriter.php 92.7% <0%> (+6.25%) 34% <0%> (ø) ⬇️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfd797e...9e6ee12. Read the comment docs.

@individual-it
individual-it force-pushed the ui-tests-for-ldap branch 2 times, most recently from 72a8704 to 8b172c5 Compare January 31, 2018 11:05
@individual-it

Copy link
Copy Markdown
Member Author

@ownclouders rebase

@ownclouders

Copy link
Copy Markdown
Contributor

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

@ownclouders

Copy link
Copy Markdown
Contributor

Automated rebase with GitMate.io was successful! 🎉

@individual-it individual-it changed the title [WIP] using LDAP users in UI tests using LDAP users in UI tests Feb 2, 2018
@individual-it individual-it added this to the development milestone Feb 2, 2018
@individual-it

Copy link
Copy Markdown
Member Author

cleaned stuff up, please review

@phil-davis phil-davis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It will work as it is, but there are a few comments about the logic to handle the occ method.

//guess yourself
$method = "ldap";
} else {
$method = "api";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If $method is passed in here as occ (for example) then this code will go down this "else" and set (override) $method to be api

I don't see how method occ can be used?

}
break;
case "ldap":
echo "creating LDAP users is not implemented, so assume they exists\n";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/they exists/they exist

//guess yourself
$method = "ldap";
} else {
$method = "api";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here - how to use method occ ?

}
break;
case "ldap":
echo "creating LDAP groups is not implemented, so assume they exists\n";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/they exists/they exist

//guess yourself
$method = "ldap";
} else {
$method = "api";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here - how to use method occ ?

@individual-it

Copy link
Copy Markdown
Member Author

@phil-davis fixed the mentioned issues

@individual-it
individual-it merged commit faaaa42 into master Feb 3, 2018
@individual-it

Copy link
Copy Markdown
Member Author

backport in #30349

@lock

lock Bot commented Aug 1, 2019

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock Bot locked as resolved and limited conversation to collaborators Aug 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants