Skip to content

make GitLabConnection describable and add jcasc test#1063

Merged
markjacksonfishing merged 3 commits into
jenkinsci:masterfrom
jetersen:fix/jcasc-compat
Jun 3, 2020
Merged

make GitLabConnection describable and add jcasc test#1063
markjacksonfishing merged 3 commits into
jenkinsci:masterfrom
jetersen:fix/jcasc-compat

Conversation

@jetersen

@jetersen jetersen commented May 17, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Made GitLabConnection describable per Jenkins/JCasC standards and added JCasC tests also confirmed in UI that everything is working as expected.

fixes #1062

@markjacksonfishing

markjacksonfishing commented May 17, 2020

Copy link
Copy Markdown
Contributor

Thank you @jetersen will take a look later tonight and merge

jetersen
jetersen commented May 17, 2020
View reviewed changes
Comment thread src/main/java/com/dabsquared/gitlabjenkins/connection/GitLabConnection.java
@Restricted(NoExternalUse.class)
private String getApiToken(String apiTokenId, Item item) {
ItemGroup context = null != item ? item.getParent() : Jenkins.getInstance();
ItemGroup<?> context = null != item ? item.getParent() : Jenkins.get();

@jetersen jetersen May 17, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Gratuitous change

Comment thread src/main/java/com/dabsquared/gitlabjenkins/connection/GitLabConnection.java
@Initializer(after = InitMilestone.PLUGINS_STARTED)
public static void migrate() throws IOException {
GitLabConnectionConfig descriptor = (GitLabConnectionConfig) Jenkins.getInstance().getDescriptor(GitLabConnectionConfig.class);
GitLabConnectionConfig descriptor = (GitLabConnectionConfig) Jenkins.get().getDescriptor(GitLabConnectionConfig.class);

@jetersen jetersen May 17, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Gratuitous change

Comment thread src/main/java/com/dabsquared/gitlabjenkins/connection/GitLabConnectionConfig.java
public FormValidation doCheckName(@QueryParameter String id, @QueryParameter String value) {
if (StringUtils.isEmptyOrNull(value)) {
return FormValidation.error(Messages.name_required());
} else if (connectionMap.containsKey(value) && !connectionMap.get(value).toString().equals(id)) {

@jetersen jetersen May 17, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This check is very hard to do, even prior to my changes this will only work after initial save.
Not even sure why a transient map is used for lookups.

Comment thread src/main/resources/com/dabsquared/gitlabjenkins/connection/GitLabConnectionConfig/config.jelly
@@ -1,46 +1,17 @@
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form" xmlns:c="/lib/credentials" xmlns:st="jelly:stapler">
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">

@jetersen jetersen May 17, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Gratuitous change

@jetersen

jetersen commented May 17, 2020

Copy link
Copy Markdown
Member Author

Build failure seems unrelated.

020-05-17T21:59:44.933Z] #

[2020-05-17T21:59:44.933Z] # There is insufficient memory for the Java Runtime Environment to continue.

[2020-05-17T21:59:44.933Z] # Native memory allocation (mmap) failed to map 54525952 bytes for committing reserved memory.

[2020-05-17T21:59:44.933Z] SUREFIRE-859: OpenJDK 64-Bit Server VM warning: INFO: os::commit_memory(0x00000000c7100000, 54525952, 0) failed; error='Cannot allocate memory' (errno=12)

That's a new one...

@jetersen jetersen closed this May 17, 2020
@jetersen jetersen reopened this May 17, 2020
@markjacksonfishing

markjacksonfishing commented May 17, 2020

Copy link
Copy Markdown
Contributor

This could be a problem with the infra

@jetersen

jetersen commented May 17, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

Windows machine in the second run took almost 40 minutes to run the tests and obviously failed due to disk issues.

@jetersen jetersen closed this May 17, 2020
@jetersen jetersen reopened this May 17, 2020
@jetersen

jetersen commented May 17, 2020

Copy link
Copy Markdown
Member Author

flaky windows tests as ever...

@jetersen jetersen closed this May 17, 2020
@jetersen jetersen reopened this May 17, 2020
@jetersen jetersen mentioned this pull request May 18, 2020
Closed
3 tasks
@jetersen

jetersen commented May 19, 2020

Copy link
Copy Markdown
Member Author

@markyjackson-taulia any chance disabling some of the windows tests that require disks?

@jetersen jetersen closed this May 19, 2020
@markjacksonfishing

markjacksonfishing commented May 19, 2020

Copy link
Copy Markdown
Contributor

@markyjackson-taulia any chance disabling some of the windows tests that require disks?

@jetersen Let me take a look at doing that

@jetersen jetersen reopened this May 19, 2020
@jetersen

jetersen commented May 19, 2020

Copy link
Copy Markdown
Member Author

Ya failed disk tests again, I give up... trying to get jenkins tests to pass.

@oleg-nenashev oleg-nenashev self-requested a review May 20, 2020 08:12
@jetersen jetersen mentioned this pull request May 25, 2020
Merged
3 tasks
@jetersen

jetersen commented Jun 3, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

gitlab-plugin/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/pipeline/PipelineHookTriggerHandlerImplTest.java

Line 66 in 79a806b

Git git = Git.open(tmp.getRoot());

This should properly close after the run.

jetersen added 2 commits June 3, 2020 11:20
@jetersen
remember to close git repo
ccd96b6
@jetersen
modernise...
5eb82cd 
all the mocked tests will never work without rewrite
@jetersen

jetersen commented Jun 3, 2020

Copy link
Copy Markdown
Member Author

Oh dear @markyjackson-taulia this plugin pom was terrible old, and causing the Linux agent to crash due requesting too much memory. This is fixed by updating parent pom and Jenkins test harness...
Though that breaks all the mocked jenkins jobs inside the tests...

Last I heard mocked jobs is a big nono 🗡️

So ccd96b6 fixed the temporary folder cleanup...
while 5eb82cd breaks all mocked jobs 😭

@markjacksonfishing markjacksonfishing merged commit f1c9907 into jenkinsci:master Jun 3, 2020
@jetersen jetersen deleted the fix/jcasc-compat branch June 3, 2020 14:47
@jetersen

jetersen commented Jun 3, 2020

Copy link
Copy Markdown
Member Author

Thanks for merging. Much appreciated @markyjackson-taulia

@timja

timja commented Dec 31, 2020

Copy link
Copy Markdown
Member

@markyjackson-taulia could you release this please?

@markjacksonfishing

markjacksonfishing commented Dec 31, 2020

Copy link
Copy Markdown
Contributor

Will do it over the weekend

@timja

timja commented Jan 11, 2021

Copy link
Copy Markdown
Member

@markyjackson-taulia reminder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oleg-nenashev oleg-nenashev Awaiting requested review from oleg-nenashev
1 more reviewer
@markjacksonfishing markjacksonfishing markjacksonfishing approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

empty GitLab connection is created in addition the one defined in jenkins.yml

3 participants

@jetersen @markjacksonfishing @timja