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

Merge master into BETA_JAVA23 #2888

Merged
merged 0 commits into from
Sep 3, 2024
Merged

Conversation

jarthana
Copy link
Member

@jarthana jarthana commented Sep 3, 2024

What it does

How to test

Author checklist

@jarthana jarthana merged commit 17d4d2d into eclipse-jdt:BETA_JAVA23 Sep 3, 2024
2 of 6 checks passed
@stephan-herrmann
Copy link
Contributor

@jarthana it looks like this force-pushed killed some prior changes on BETA_JAVA23!!

I happened to check for the changes from PR #2881

Could this have happened to other changes, too? How to figure out?

I found one interesting diff in https://github.com/eclipse-jdt/eclipse.jdt.core/compare/f37596e415196f39ac7bbf148e03ff68938537ed..17d4d2dfcaf62f120575753e1a6765530f84f37f#diff-8a6569d250cda98263a9f33412e161158a279fa6b9e4844afc474c3bee4ab083

Given that 17d4d2d is current HEAD in BETA_JAVA23 it looks like we lost even more changes, e.g., #2878 seems to be affected, too?

I locally still have my 0853e11 from #2881, will try to create a PR to restore all that was in there.

@jarthana , @mpalat Please don't merge to BETA_JAVA23 for the time being, so my chances to restore lost changes are kept.

@jarthana
Copy link
Member Author

jarthana commented Sep 3, 2024

Oh my. I was pretty sure, or so I thought, that I had pulled all the changes from the BETA_JAVA23. I have no idea how they went missing. I have no plans to do anything more on this. Please let me know if I can offer some help here.

@jarthana
Copy link
Member Author

jarthana commented Sep 3, 2024

Could this have happened to other changes, too? How to figure out?

This merge only brought in 4 commits and handful of changes to the BETA_JAVA23. Assuming your local BETA branch is still good, doing a comparison between this and your branch should tell us.

@stephan-herrmann
Copy link
Contributor

The merge claims as its parent: 3756a1a

This dropped at least the following commits (quoted from a branch in my local clone):

How can we find out if that list is complete? At least from the list of closed MRs, #2881 is indeed that latest on BETA_JAVA23 prior to the merge. Unfortunately, the merge of #2881 changed hashes wrt my local branch, so there's a tiny chance that my local branch is not exactly what we had on github, but so be it.

Here's my plan:

  • in one local branch re-apply (cherry pick) the above commits in order
  • in a second branch re-do merge from master on top of my latest in BETA_JAVA23
  • compare both branches
    • if no significant changes found, I'll submit the former branch without squashing.
    • otherwise we'll scratch our heads

@jarthana
Copy link
Member Author

jarthana commented Sep 3, 2024

Here's my plan:

* in one local branch re-apply (cherry pick) the above commits in order
* in a second branch re-do merge from master on top of my latest in BETA_JAVA23
* compare both branches
  
  * if no significant changes found, I'll submit the former branch without squashing.
  * otherwise we'll scratch our heads

How about going with the latter? Assuming no change went into BETA since you last pulled, we are safer that way. Betting on my merge sounds flaky, imo.

@stephan-herrmann
Copy link
Contributor

Here's my plan:

* in one local branch re-apply (cherry pick) the above commits in order

that's #2892

@stephan-herrmann
Copy link
Contributor

Here's my plan:

  • [...]
  • in a second branch re-do merge from master on top of my latest in BETA_JAVA23

this merge succeeded without any conflicts

  • compare both branches

Git Tree Compare tells me: "No differences found ..." 👍

@stephan-herrmann
Copy link
Contributor

Here's my plan:

* in one local branch re-apply (cherry pick) the above commits in order
* in a second branch re-do merge from master on top of my latest in BETA_JAVA23
* compare both branches
  
  * if no significant changes found, I'll submit the former branch without squashing.
  * otherwise we'll scratch our heads

How about going with the latter? Assuming no change went into BETA since you last pulled, we are safer that way. Betting on my merge sounds flaky, imo.

The advantage of using your merge + cherry pick: that branch can be submitted without yet another force push.

The local comparison between both branches tells us we should have the best safety which my local branch can provide.

@stephan-herrmann
Copy link
Contributor

As a second way of verifying I was going to check the number of tests run, but then I can't even find a single check linked from this PR, how is this possible?

@stephan-herrmann
Copy link
Contributor

#2881 lists: "passed: 97274" - so that's the minimum we should reach also in #2892.

Did master introduce new tests in the relevant period?

@stephan-herrmann
Copy link
Contributor

Did master introduce new tests in the relevant period?

I believe this is the list of PRs from master going into the merge:

@jarthana can you confirm this list?

@jarthana
Copy link
Member Author

jarthana commented Sep 3, 2024

#2881 lists: "passed: 97274" - so that's the minimum we should reach also in #2892.

Did master introduce new tests in the relevant period?

I see two new tests in master (430761c and be7e2a9)

@stephan-herrmann
Copy link
Contributor

As of #2770 jenkins reported passed: 96524 -- this should be the last commit from the previous merge from master

As of #2702 we had passed: 96524, ups?

lesson learned: if the PR build was on top of old state in master, then it doesn't really tell us what I'm asking here ...

@stephan-herrmann
Copy link
Contributor

Trying a different route:

6 additional tests (including tests run at multiple compliance levels)

Do PR builds and master builds use the same set of compliance levels?

@jarthana
Copy link
Member Author

jarthana commented Sep 3, 2024

Do PR builds and master builds use the same set of compliance levels?

Probably not. I see far fewer tests on the I build.

@stephan-herrmann
Copy link
Contributor

Do PR builds and master builds use the same set of compliance levels?

PR build (master):

Probably not. I see far fewer tests on the I build.

I wasn't referring to I-build but to https://ci.eclipse.org/jdt/job/eclipse.jdt.core-Github/job/master/

@jarthana
Copy link
Member Author

jarthana commented Sep 3, 2024

I see five instances of testMissingClass_varargs4_noArg (here) and one instance of other. So, 6 accounted for.

@stephan-herrmann
Copy link
Contributor

https://ci.eclipse.org/jdt/job/eclipse.jdt.core-Github/job/PR-2892/1/testReport/ : 97,280 tests

Previous on beta: passed: 97274

Expected increase: 6

Expected total: 97280

@jarthana I'll retrigger jenkins until we're free from intermittent failures.

Please confirm that this can be submitted once we have a green build.

@jarthana
Copy link
Member Author

jarthana commented Sep 3, 2024

Failed again with a different test in the same suite. Kicked off again. Let's see if third time's lucky.

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