OpenLiberty / liberty-tools-intellij

IntelliJ IDEA extension for Liberty
https://plugins.jetbrains.com/plugin/14856-open-liberty-tools
Eclipse Public License 2.0
11 stars 20 forks source link

Run LSP4iJ continuous integration builds with existing LTI releases. #815

Open mrglavas opened 3 weeks ago

mrglavas commented 3 weeks ago

As new versions of LSP4iJ are released it will be possible (and very likely) that users installing LTI will also install a later version of LSP4iJ than was tested at the time of the LTI release. We want to ensure existing releases of LTI continue to work with new LSP4iJ versions. Running continuous integration builds with existing LTI releases would help facilitate this and would hopefully catch issues introduced into LSP4iJ before the new release of LSP4iJ is published to the Marketplace.

The purpose of this work item is to ensure that we have the infrastructure in place to support these builds (starting with 24.0.7) and to define the process for including LTI releases in the workflow(s).

TrevCraw commented 2 weeks ago

Based on feedback from @yeekangc , we will look to test LTI releases from the previous 6 months. I would suggest that if there is only one previous release in the past 6 months, we should test one more previous release so that we are always testing at least 2 releases back.

mrglavas commented 2 weeks ago

I'm imagining that which of the previous LTI releases are tested would be controlled by configuration to the workflow, in other words that we would specify exactly which releases to test and not have the workflow dynamically figure out which releases are in the 6-month window.

vaisakhkannan commented 1 week ago

Different approaches to implement the solution:

  1. Fetch the current main code of LSP4IJ and build it with existing LTI Releases to check the build results.
  2. Fetch the PR merge commits of LSP4IJ and build it with existing LTI Releases to check the build results.
  3. Fetch nightly builds of LSP4IJ from the marketplace and buildit with existing LTI Releases to check the build results.

cc: @TrevCraw @mrglavas

vaisakhkannan commented 1 week ago

I have added specifications for our existing LTI release tags, 24.0.3 and 24.0.6, as shown in the image below.

Screenshot 2024-06-21 at 3 08 13 PM

I can see the 'Build Liberty-Tools-Intellij' step is failing. We know this is an expected behaviour ,

Screenshot 2024-06-21 at 1 37 38 PM Screenshot 2024-06-21 at 1 37 50 PM

For Reference : https://github.com/vaisakhkannan/liberty-tools-intellij/actions/runs/9611377420/job/26509811344

TrevCraw commented 1 week ago

Different approaches to implement the solution:

  1. Fetch the current main code of LSP4IJ and build it with existing LTI Releases to check the build results.
  2. Fetch the PR merge commits of LSP4IJ and build it with existing LTI Releases to check the build results.
  3. Fetch nightly builds of LSP4IJ from the marketplace and buildit with existing LTI Releases to check the build results.

cc: @TrevCraw @mrglavas

@mrglavas What direction do you think would be the best to go for these builds?

mrglavas commented 1 week ago

@TrevCraw @vaisakhkannan Considering we're already building for every PR on our current development on main, I'm leaning towards option 2 for consistency. I think this would make it easier to compare build results and to assess the impact of a specific change made to LSP4iJ across all of the versions of LTI we have in scope, including our current development stream. I realize that puts a n + 1 multiplier (where n is the number of existing LTI releases we currently care about) on the total number of continuous integration builds we're doing. Whomever is monitoring would be reading all of those results. I'm curious if folks think that's too much.

vaisakhkannan commented 1 week ago

@mrglavas @TrevCraw As of now, we do not have any existing releases that use the LSP4IJ plugin to test. So, I created a branch #815-lsp4ij-ci-build-existing-LTI which includes the changes from Anusree’s PR: https://github.com/OpenLiberty/liberty-tools-intellij/pull/834. I took these changes and created two new branches that include the PR changes. Then, I created two tags in my fork, namely v0.0.0.1 and v0.0.0.2. I specified these tags in an array in my build.yaml file and tested the code using these tags, resulting in successful builds.

Similarly, I checked with Anusree about running the LTI Tests against each open LSP4IJ PR. Those changes are present in this PR: https://github.com/OpenLiberty/liberty-tools-intellij/pull/839.

I did the testing based on the specified tags which ran against each open LSP4IJ PR. Since we can’t test against existing LTI, this helped me get the build to run successfully. Therefore, we can use this approach (Option 2).

Screenshot 2024-06-24 at 6 28 11 PM

Based on Michael’s comment above, we should consider an n + 1 multiplier for continuous integration builds we're doing. We can specify 'n' number of tags, and similarly, '1' represents the main branch changes. Anusree has the PR https://github.com/OpenLiberty/liberty-tools-intellij/pull/839, which includes the changes to run against the main branch (Option 1).

I hope there is no need to open a PR for Option 2 for now, since we don't have an existing LTI release that uses the LSP4IJ plugin. The changes I have made are present in the above Anusree's PR. I just modified the code in two lines (Line 33 and Line 60) as shown in the screenshot below.

Screenshot 2024-06-24 at 5 42 47 PM

Do you think I need to investigate further, or should I check Option 3 that I provided? Feel free to add your suggestions as well; I can check.

TrevCraw commented 1 week ago

Hi @vaisakhkannan, after a discussion with @mrglavas, we agreed that it is good to move forward with what you have here. We should also run against the main branch for LSP4IJ, but that part will be covered by Anusree's changes.

Once Anusree merges her changes, we should open a PR with updates for including the matrix with the branches and using the matrix.tag.

TrevCraw commented 1 week ago

One thing I wanted to note in this issue was the importance of having clean builds moving forward. If we are planning to test old tagged versions in the repo, we should ensure that the tests are running smooth with green builds at the time the tag is created.