Skip to content
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.

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

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

Closed
mrglavas opened this issue Jun 12, 2024 · 9 comments
Closed
Assignees
Milestone

Comments

@mrglavas
Copy link
Contributor

mrglavas commented Jun 12, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

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 vaisakhkannan self-assigned this Jun 20, 2024
@vaisakhkannan
Copy link
Contributor

vaisakhkannan commented Jun 21, 2024

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
Copy link
Contributor

vaisakhkannan commented Jun 21, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

mrglavas commented Jun 21, 2024

@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
Copy link
Contributor

vaisakhkannan commented Jun 24, 2024

@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: #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: #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 #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
Copy link
Contributor

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
Copy link
Contributor

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.

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

No branches or pull requests

3 participants