Skip to content

Feature/protected branch and example test properties #156

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Mar 12, 2018
Merged

Feature/protected branch and example test properties #156

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c2186d4
Initial commit.
gmessner Mar 12, 2018
44eb844
Minor mods to protected branch functionality.
gmessner Mar 12, 2018
d4906b7
Cleaned up example-test-gitlab4j.properties and releated tests.
gmessner Mar 12, 2018
1db1f77
Fixed test that was accidentally broken.
gmessner Mar 12, 2018
cac5f11
Now verifies branch name of returned protected branch.
gmessner Mar 12, 2018
5847870
Improved consistency and made comments clearer.
gmessner Mar 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions example-test-gitlab4j.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,33 @@
# Rename this file to test-gitlab4j.properties with proper property values.
# To test GitLab4J-API you must have access to a gitlab server.
#
# Copy this file to test-gitlab4j.properties and update the property
# values to match the gitlab server you are testing against.
#
# The TEST_PRIVATE_TOKEN and TEST_USERNAME should belong to an admin user
#

# The following values must be set or testing will fail
TEST_NAMESPACE=some-namespace
TEST_PROJECT_NAME=some-project-name
TEST_HOST_URL=some-gitlab-url
TEST_PRIVATE_TOKEN=some-private-token
TEST_PRIVATE_TOKEN=admin-private-token
TEST_ACCESS_TOKEN=some-access-token
TEST_USERNAME=admin-user

# To test the GroupApi, set these to an existing group and group-project, and group member
TEST_GROUP=
TEST_GROUP_PROJECT=
TEST_GROUP_MEMBER_USERNAME=

# To test oauth2Login, set these properties
TEST_LOGIN_USERNAME=
TEST_LOGIN_PASSWORD=

# To test sudo capability provide a username to sudo as
TEST_SUDO_AS_USERNAME=

# To test using GitLab4J-API with a proxy, set the following properties
TEST_PROXY_URI=
TEST_PROXY_USERNAME=
TEST_PROXY_PASSWORD=
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we follow the above convention of having some stub string such as some-proxy-password?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As you imply, to be consistent it should be one way or the other. Providing a value could lead to some confusion, (are these valid values?), so I suggest we remove all values, something like this:

#
# To test GitLab4J-API, you must have access to a GitLab server.
#
# Copy this file to test-gitlab4j.properties and set the property
# values to match the GitLab server you are testing against.
#

# REQUIRED: The following values must be set or the tests will fail.
# The TEST_PRIVATE_TOKEN and TEST_USERNAME must belong to an admin user. 
TEST_PRIVATE_TOKEN=
TEST_ACCESS_TOKEN=
TEST_USERNAME=
TEST_NAMESPACE=
TEST_PROJECT_NAME=
TEST_HOST_URL=

# OPTIONAL: To test the GroupApi, set these to an existing group name and group-project name,
# and the username of a group member
TEST_GROUP=
TEST_GROUP_PROJECT=
TEST_GROUP_MEMBER_USERNAME=

# OPTIONAL: To test oauth2Login, set these properties
TEST_LOGIN_USERNAME=
TEST_LOGIN_PASSWORD=

# OPTIONAL: To test sudo capability provide a username to sudo as
TEST_SUDO_AS_USERNAME=

# OPTIONAL: To test using GitLab4J-API with a proxy, set the following properties
TEST_PROXY_URI=
TEST_PROXY_USERNAME=
TEST_PROXY_PASSWORD=

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll push the above unless you have a very reason to do it differently.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with what you said about validity. My comment was more so on picking one convention over another.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I figured that, I just pushed this change.


1 change: 1 addition & 0 deletions src/main/java/org/gitlab4j/api/ProtectedBranchesApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.List;

public class ProtectedBranchesApi extends AbstractApi {

public ProtectedBranchesApi(GitLabApi gitLabApi) {
super(gitLabApi);
}
Expand Down
1 change: 0 additions & 1 deletion src/main/java/org/gitlab4j/api/RepositoryApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import javax.ws.rs.core.Response;

import org.gitlab4j.api.GitLabApi.ApiVersion;
import org.gitlab4j.api.models.AccessLevel;
import org.gitlab4j.api.models.Branch;
import org.gitlab4j.api.models.CompareResults;
import org.gitlab4j.api.models.Tag;
Expand Down
9 changes: 5 additions & 4 deletions ...j/api/models/BranchAccessLevelDetail.java → ...itlab4j/api/models/BranchAccessLevel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@

@XmlRootElement
@XmlAccessorType(XmlAccessType.FIELD)
public class BranchAccessLevelDetail {
private String accessLevel;
public class BranchAccessLevel {

private AccessLevel accessLevel;
private String accessLevelDescription;

public String getAccessLevel() {
public AccessLevel getAccessLevel() {
return this.accessLevel;
}

public void setAccessLevel(String accessLevel) {
public void setAccessLevel(AccessLevel accessLevel) {
this.accessLevel = accessLevel;
}

Expand Down
13 changes: 7 additions & 6 deletions src/main/java/org/gitlab4j/api/models/ProtectedBranch.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@
@XmlRootElement
@XmlAccessorType(XmlAccessType.FIELD)
public class ProtectedBranch {

private String name;
private List<BranchAccessLevelDetail> pushAccessLevels;
private List<BranchAccessLevelDetail> mergeAccessLevels;
private List<BranchAccessLevel> pushAccessLevels;
private List<BranchAccessLevel> mergeAccessLevels;

public String getName() {
return this.name;
Expand All @@ -21,19 +22,19 @@ public void setName(String name) {
this.name = name;
}

public List<BranchAccessLevelDetail> getPushAccessLevels() {
public List<BranchAccessLevel> getPushAccessLevels() {
return this.pushAccessLevels;
}

public void setPushAccessLevels(List<BranchAccessLevelDetail> pushAccessLevels) {
public void setPushAccessLevels(List<BranchAccessLevel> pushAccessLevels) {
this.pushAccessLevels = pushAccessLevels;
}

public List<BranchAccessLevelDetail> getMergeAccessLevels() {
public List<BranchAccessLevel> getMergeAccessLevels() {
return this.mergeAccessLevels;
}

public void setMergeAccessLevels(List<BranchAccessLevelDetail> mergeAccessLevels) {
public void setMergeAccessLevels(List<BranchAccessLevel> mergeAccessLevels) {
this.mergeAccessLevels = mergeAccessLevels;
}

Expand Down
1 change: 1 addition & 0 deletions src/test/java/org/gitlab4j/api/TestGitLabApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public void testGetVersion() throws GitLabApiException {
@Test
public void testProxyConnection() throws GitLabApiException {
assumeTrue(TEST_PROXY_URI != null && TEST_PROXY_USERNAME != null && TEST_PROXY_PASSWORD != null);
assumeTrue(TEST_PROXY_URI.length() > 0 && TEST_PROXY_USERNAME.length() > 0 && TEST_PROXY_PASSWORD.length() > 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest using TEST_PROXY_URI.isEmpty() for length checks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TEST_PROXY_URI.length() > 0
vs
!TEST_PROXY_URI.isEmpty()

When checking for truth, I prefer to use a statement that results in true, not something that has to be negated to produce true. It's just a personal preference and is used throughout GitLab4J, so to change it in these instances would require a sweep through all the code to be consistent.

Copy link
Contributor

Choose a reason for hiding this comment

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

The reasoning sounds good to me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@lamdav
FYI, when checking for empty instead of not empty I should be using isEmpty(), so I have gone through all the tests and replaced str.size() == 0 with str.isEmpty(), just thought I would let you know.


// Setup a GitLabApi instance to use a proxy
Map<String, Object> clientConfig = ProxyClientConfig.createProxyClientConfig(TEST_PROXY_URI, TEST_PROXY_USERNAME, TEST_PROXY_PASSWORD);
Expand Down
12 changes: 12 additions & 0 deletions src/test/java/org/gitlab4j/api/TestGitLabApiBeans.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import org.gitlab4j.api.models.Project;
import org.gitlab4j.api.models.ProjectHook;
import org.gitlab4j.api.models.ProjectUser;
import org.gitlab4j.api.models.ProtectedBranch;
import org.gitlab4j.api.models.Session;
import org.gitlab4j.api.models.Snippet;
import org.gitlab4j.api.models.SshKey;
Expand Down Expand Up @@ -266,6 +267,17 @@ public void testProjectHook() {
}
}

@Test
public void testProtectedBranch() {

try {
ProtectedBranch protectedBranch = makeFakeApiCall(ProtectedBranch.class, "protected-branch");
assertTrue(compareJson(protectedBranch, "protected-branch"));
} catch (Exception e) {
e.printStackTrace();
}
}

@Test
public void testKey() {

Expand Down
28 changes: 14 additions & 14 deletions src/test/java/org/gitlab4j/api/TestGitLabLogin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,22 @@
* In order for these tests to run you must set the following properties in test-gitlab4j.properties
*
* TEST_HOST_URL
* TEST_USERNAME
* TEST_PASSWORD
* TEST_LOGIN_USERNAME
* TEST_LOGIN_PASSWORD
* TEST_PRIVATE_TOKEN
*
* If any of the above are NULL, all tests in this class will be skipped.
*/
public class TestGitLabLogin {

// The following needs to be set to your test repository
private static final String TEST_USERNAME;
private static final String TEST_PASSWORD;
private static final String TEST_LOGIN_USERNAME;
private static final String TEST_LOGIN_PASSWORD;
private static final String TEST_HOST_URL;
private static final String TEST_PRIVATE_TOKEN;
static {
TEST_USERNAME = TestUtils.getProperty("TEST_USERNAME");
TEST_PASSWORD = TestUtils.getProperty("TEST_PASSWORD");
TEST_LOGIN_USERNAME = TestUtils.getProperty("TEST_LOGIN_USERNAME");
TEST_LOGIN_PASSWORD = TestUtils.getProperty("TEST_LOGIN_PASSWORD");
TEST_HOST_URL = TestUtils.getProperty("TEST_HOST_URL");
TEST_PRIVATE_TOKEN = TestUtils.getProperty("TEST_PRIVATE_TOKEN");
}
Expand All @@ -47,12 +47,12 @@ public static void setup() {

problems = "";

if (TEST_USERNAME == null || TEST_USERNAME.trim().length() == 0) {
problems += "TEST_USERNAME cannot be empty\n";
if (TEST_LOGIN_USERNAME == null || TEST_LOGIN_USERNAME.trim().length() == 0) {
problems += "TEST_LOGIN_USERNAME cannot be empty\n";
}

if (TEST_PASSWORD == null || TEST_PASSWORD.trim().length() == 0) {
problems += "TEST_PASSWORD cannot be empty\n";
if (TEST_LOGIN_PASSWORD == null || TEST_LOGIN_PASSWORD.trim().length() == 0) {
problems += "TEST_LOGIN_PASSWORD cannot be empty\n";
}

if (TEST_HOST_URL == null || TEST_HOST_URL.trim().length() == 0) {
Expand Down Expand Up @@ -93,7 +93,7 @@ public void beforeMethod() {
public void testSession() throws GitLabApiException {

assumeTrue(hasSession);
GitLabApi gitLabApi = GitLabApi.login(ApiVersion.V4, TEST_HOST_URL, TEST_USERNAME, TEST_PASSWORD);
GitLabApi gitLabApi = GitLabApi.login(ApiVersion.V4, TEST_HOST_URL, TEST_LOGIN_USERNAME, TEST_LOGIN_PASSWORD);
assertNotNull(gitLabApi);
assertNotNull(gitLabApi.getSession());
assertEquals(TEST_PRIVATE_TOKEN, gitLabApi.getSession().getPrivateToken());
Expand All @@ -104,7 +104,7 @@ public void testSession() throws GitLabApiException {
public void testSessionV3() throws GitLabApiException {

assumeTrue(hasSession);
GitLabApi gitLabApi = GitLabApi.login(ApiVersion.V3, TEST_HOST_URL, TEST_USERNAME, TEST_PASSWORD);
GitLabApi gitLabApi = GitLabApi.login(ApiVersion.V3, TEST_HOST_URL, TEST_LOGIN_USERNAME, TEST_LOGIN_PASSWORD);
assertNotNull(gitLabApi);
assertNotNull(gitLabApi.getSession());
assertEquals(TEST_PRIVATE_TOKEN, gitLabApi.getSession().getPrivateToken());
Expand All @@ -114,7 +114,7 @@ public void testSessionV3() throws GitLabApiException {
public void testSessionFallover() throws GitLabApiException {

assumeFalse(hasSession);
GitLabApi gitLabApi = GitLabApi.login(ApiVersion.V4, TEST_HOST_URL, TEST_USERNAME, TEST_PASSWORD);
GitLabApi gitLabApi = GitLabApi.login(ApiVersion.V4, TEST_HOST_URL, TEST_LOGIN_USERNAME, TEST_LOGIN_PASSWORD);
assertNotNull(gitLabApi);
Version version = gitLabApi.getVersion();
assertNotNull(version);
Expand All @@ -123,7 +123,7 @@ public void testSessionFallover() throws GitLabApiException {
@Test
public void testOauth2Login() throws GitLabApiException {

GitLabApi gitLabApi = GitLabApi.oauth2Login(TEST_HOST_URL, TEST_USERNAME, TEST_PASSWORD, null, null, true);
GitLabApi gitLabApi = GitLabApi.oauth2Login(TEST_HOST_URL, TEST_LOGIN_USERNAME, TEST_LOGIN_PASSWORD, null, null, true);
assertNotNull(gitLabApi);
Version version = gitLabApi.getVersion();
assertNotNull(version);
Expand Down
10 changes: 6 additions & 4 deletions src/test/java/org/gitlab4j/api/TestGroupApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,12 @@ public static void setup() {
problems += "Problem fetching test group, error=" + gle.getMessage() + "\n";
}

try {
testUser = gitLabApi.getUserApi().getUser(TEST_GROUP_MEMBER_USERNAME);
} catch (GitLabApiException gle) {
problems += "Problem fetching test user, error=" + gle.getMessage() + "\n";
if (TEST_GROUP_MEMBER_USERNAME != null && TEST_GROUP_MEMBER_USERNAME.length() > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

if (TEST_GROUP_MEMBER_USERNAME != null && !TEST_GROUP_MEMBER_USERNAME.isEmpty()) { ... }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See my response above

try {
testUser = gitLabApi.getUserApi().getUser(TEST_GROUP_MEMBER_USERNAME);
} catch (GitLabApiException gle) {
problems += "Problem fetching test user, error=" + gle.getMessage() + "\n";
}
}
}

Expand Down
15 changes: 7 additions & 8 deletions src/test/java/org/gitlab4j/api/TestProtectedBranchesApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
package org.gitlab4j.api;

import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;

import java.util.List;

import org.gitlab4j.api.models.Branch;
import org.gitlab4j.api.models.Project;
import org.gitlab4j.api.models.ProtectedBranch;
Expand All @@ -10,13 +16,6 @@
import org.junit.Test;
import org.junit.runners.MethodSorters;

import java.util.List;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;

/**
* In order for these tests to run you must set the following properties in test-gitlab4j.properties
*
Expand Down Expand Up @@ -157,6 +156,6 @@ public void testProtectBranch() throws GitLabApiException {
List<ProtectedBranch> branches = gitLabApi.getProtectedBranchesApi().getProtectedBranches(project.getId());
assertNotNull(branches);
assertTrue(branches.stream()
.anyMatch((protectedBranch) -> protectedBranch.getName().equals(TEST_BRANCH_NAME)));
.anyMatch((protectedBranch) -> branch.getName().equals(TEST_BRANCH_NAME)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct me if I am wrong, but I think this should remain unchanged. I wrote this test to protect an unprotected branch, get a list of protected branches, and verify that the unprotected branch is now part of this list.

By changing the name, the assertion will always return true as branch is effectively treated as a final variable (line 155) and should always be TEST_BRANCH_NAME by the time (line 159) executes. If it does not, (line 155) should have failed with an exception thrown.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure how that got changed, fixed and pushed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Made another change to this. After the revert to protectedBranch the branch variable was not referenced, and I realized that the contents of the returned protected branch were never really validated, by validating the name, it took care of the unused reference.

}
}
3 changes: 2 additions & 1 deletion src/test/java/org/gitlab4j/api/TestRepositoryApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,14 @@ public static void setup() {

if (problems.isEmpty()) {
gitLabApi = new GitLabApi(ApiVersion.V3, TEST_HOST_URL, TEST_PRIVATE_TOKEN);
teardown();
} else {
System.err.print(problems);
}
}

@AfterClass
public static void teardown() throws GitLabApiException {
public static void teardown() {
if (gitLabApi != null) {

try {
Expand Down
7 changes: 3 additions & 4 deletions src/test/java/org/gitlab4j/api/TestSystemHooksApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@

/**
* In order for these tests to run you must set the following properties in test-gitlab4j.properties
*
* TEST_HOOK_URL
*
* TEST_HOST_URL
* TEST_PRIVATE_TOKEN
*
*
* If any of the above are NULL, all tests in this class will be skipped.
*/
@FixMethodOrder(MethodSorters.NAME_ASCENDING)
Expand All @@ -34,7 +33,7 @@ public class TestSystemHooksApi {
private static final String TEST_PRIVATE_TOKEN;
static {
TEST_HOST_URL = TestUtils.getProperty("TEST_HOST_URL");
TEST_HOOK_URL = TestUtils.getProperty("TEST_HOOK_URL");
TEST_HOOK_URL = "http://hook.example.com/hook/callback";
TEST_PRIVATE_TOKEN = TestUtils.getProperty("TEST_PRIVATE_TOKEN");
}

Expand Down
12 changes: 8 additions & 4 deletions src/test/java/org/gitlab4j/api/TestUserApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@

/**
* In order for these tests to run you must set the following properties in test-gitlab4j.properties
*
*
* TEST_HOST_URL
* TEST_PRIVATE_TOKEN
* TEST_USERNAME
*
*
* If any of the above are NULL, all tests in this class will be skipped.
*
* TEST_SUDO_AS_USERNAME
Expand All @@ -56,7 +56,11 @@ public class TestUserApi {
TEST_PRIVATE_TOKEN = TestUtils.getProperty("TEST_PRIVATE_TOKEN");
TEST_USERNAME = TestUtils.getProperty("TEST_USERNAME");
TEST_SUDO_AS_USERNAME = TestUtils.getProperty("TEST_SUDO_AS_USERNAME");
TEST_SSH_KEY = TestUtils.getProperty("TEST_SSH_KEY");
TEST_SSH_KEY = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCvbkmGRaANy2nmLrfYa9LkjMqjs9twYZXQKUPK18j" +
"BWmNgnAm818IikxjfFit3Gqnnh9zdNzlzUYs2osmfdHwRLeFY3hKVR6WckGYVroQuV5ArUA4+oME+IIQ2soCv/" +
"vNWfEmp2N1mpBTwi2mIYKurCKv6UpIpGK9D+ezNk5H0waVTK8EvZ/ey69Nu7C7RsbTYeyi5WY/jaUG5JbsEeKY" +
"IW/2DIlUts7gcB2hzXtt7r7+6DLx82Vb+S2jPZu2JQaB4zfgS7LQgzHUy1aAAgUUpuAbvWzuGHKO0p551Ru4qi" +
"tyXN2+OUVXcYAsuIIdGGB0wLvTDgiOOSZWnSE+sg6XX [email protected]";
}

private static final String TEST_IMPERSONATION_TOKEN_NAME = "token1";
Expand Down Expand Up @@ -149,7 +153,7 @@ public void testGetOptionalUser() throws GitLabApiException {
@Test
public void testSudoAsUser() throws GitLabApiException {

assumeTrue(TEST_SUDO_AS_USERNAME != null);
assumeTrue(TEST_SUDO_AS_USERNAME != null && TEST_SUDO_AS_USERNAME.length() > 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

isEmpty

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See my response above.


try {

Expand Down
23 changes: 23 additions & 0 deletions src/test/resources/org/gitlab4j/api/protected-branch.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"name": "develop",
"push_access_levels": [
{
"access_level": 40,
"access_level_description": "Masters"
},
{
"access_level": 30,
"access_level_description": "Developer access"
}
],
"merge_access_levels": [
{
"access_level": 40,
"access_level_description": "Masters"
},
{
"access_level": 30,
"access_level_description": "Developer access"
}
]
}