Skip to content

Conversation

PhilippeVienne
Copy link
Contributor

Refer to issue #387

@gmessner
Copy link
Collaborator

@PhilippeVienne
Thanks for your contribution.

Just one small request, I noticed there is not a bean test for the variable details. Could you add the following file to src/test/resources/org/gitlab4j/api:

project-variable-details.json

{
    "key": "TEST_VARIABLE_1",
    "variable_type": "env_var",
    "value": "TEST_1",
    "protected": false,
    "masked": true
}

And then the following test to TestGitLabApiBeans.java:

    @Test
    public void testProjectVariableDetails() throws Exception {
        Variable variable = unmarshalResource(Variable.class, "project-variable-details.json");
        assertTrue(compareJson(variable, "project-variable-details.json"));
    }

@JsonProperty("protected")
private Boolean isProtected;
@JsonProperty("masked")
private Boolean isMasked;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@PhilippeVienne
No need to name this isMasked, just name it masked and remove the @JsonProperty("masked") line.

The reason isProtected was used is because protected is a reserved keyword in Java.

assertNotNull(variable);
assertEquals(key, variable.getKey());
assertEquals(value, variable.getValue());
assertFalse(variable.getMasked());
Copy link
Collaborator

Choose a reason for hiding this comment

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

@PhilippeVienne
Not certain on this but I believe variable.getMasked() will return null in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes correct and that's why a second commit remove it

@gmessner
Copy link
Collaborator

@PhilippeVienne
Getting ready to do a release, I'm going to go ahead and merge this and then add the test I mention in my first commit. This will be included in the upcoming release.

@gmessner gmessner merged commit 1084773 into gitlab4j:master Jun 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants