jenkinsci / lockable-resources-plugin

Lock resources against concurrent use
https://plugins.jenkins.io/lockable-resources
MIT License
86 stars 183 forks source link

Reserved resources available again after reloading CasC configuration #650

Open mPokornyETM opened 4 months ago

mPokornyETM commented 4 months ago

fix #527

Testing done

Manuel tests in config page

Create jenkins instance with JCAC Reserve resource in LRM page restart jenkins Resource is still reserved

Proposed upgrade guidelines

N/A

Localizations

N/A

Submitter checklist

manosnoam commented 3 weeks ago

Thanks for this PR @mPokornyETM. Was it not merged yet due to some checks ?

mPokornyETM commented 3 weeks ago

Thanks for this PR @mPokornyETM. Was it not merged yet due to some checks ?

It has failed last time. I have no idea. It is long time ago. I merged master again into this branch. We will see if it works. WHen are the check green now, we canmerge it.

manosnoam commented 3 weeks ago

It has failed last time. I have no idea. It is long time ago. I merged master again into this branch. We will see if it works. WHen are the check green now, we canmerge it.

Checks failed on expected:<Reserved_A> but was:<null> , I think you need to update the test too: https://github.com/jenkinsci/lockable-resources-plugin/blob/f1ecb60981e246dd7a642c88dfdf9f029ad95f70/src/test/java/org/jenkins/plugins/lockableresources/ConfigurationAsCodeTest.java#L45

mPokornyETM commented 3 weeks ago

It has failed last time. I have no idea. It is long time ago. I merged master again into this branch. We will see if it works. WHen are the check green now, we canmerge it.

Checks failed on expected:<Reserved_A> but was:<null> , I think you need to update the test too:

https://github.com/jenkinsci/lockable-resources-plugin/blob/f1ecb60981e246dd7a642c88dfdf9f029ad95f70/src/test/java/org/jenkins/plugins/lockableresources/ConfigurationAsCodeTest.java#L45

thx for hit. I changed the test. Maybe now. But you are welcome to review this changes ;-)

manosnoam commented 2 weeks ago

thx for hit. I changed the test. Maybe now. But you are welcome to review this changes ;-)

I think now ConfigurationAsCodeTest.java needs a formatting fix with mvn spotless:apply