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

Update maven-verifier to 2.x #1842

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

laeubi
Copy link
Member

@laeubi laeubi commented Dec 8, 2022

No description provided.

@github-actions
Copy link

github-actions bot commented Dec 8, 2022

Test Results

356 files  ±0  356 suites  ±0   2h 18m 39s ⏱️ - 14m 5s
326 tests ±0  311 ✔️  -   5    9 💤 ±0  2 +2  4 🔥 +3 
652 runs  ±0  622 ✔️  - 10  19 💤 ±0  4 +4  7 🔥 +6 

For more details on these failures and errors, see this check.

Results for commit bc68ade. ± Comparison against base commit 95199fa.

♻️ This comment has been updated with latest results.

@laeubi
Copy link
Member Author

laeubi commented Dec 8, 2022

I got the verifier update successful now with only a few exceptions, so if anyone like to help with this it would be welcome, at the moment I have no idea what can cause this maybe something with passing the arguments.

@laeubi laeubi force-pushed the update_verifier branch 3 times, most recently from 23cadcd to a808807 Compare December 14, 2022 06:37
@laeubi
Copy link
Member Author

laeubi commented Dec 14, 2022

@akurtakov maybe something you can help with as well after M1?

I got most things fixed now but there are 5 failing test left after the upgrade to Maven Verifier 2.x I just don't understand what might cause this change in behavior .. but probably we just see a bug here that was not discovered previously?

The most notable changes for Verifier 2.x is that method names changed and arguments are no longer parsed and passed "as is" but I can't see why it should make a difference here, still a second pair of eyes might reveal something.

Next thing is that Verifier 2 executes in the same process by default instead of forking, I have disabled that so we always fork because some test don't work well with it, so it should not be an issue here.

@akurtakov
Copy link
Member

I have the will but no promises :)

@laeubi
Copy link
Member Author

laeubi commented Jan 29, 2023

Rebased...

@laeubi
Copy link
Member Author

laeubi commented May 18, 2023

Maybe one should first finish this one as a smaller increment:

@akurtakov
Copy link
Member

@dependabot rebase

@laeubi
Copy link
Member Author

laeubi commented Sep 23, 2023

@dependabot rebase

This is not a dependabot PR ;-)

I can try to rebase this and resolve conflicts but last time there where still some testfailures du to the upgrade so someone must be look into this if we want to upgrade.

@akurtakov
Copy link
Member

I would probably not find time to look into it as it's just M1 but I'll try to handle some of the deprecations done in this PR separately so it becomes less of a churn.

@laeubi laeubi added this to the 5.0 milestone Jan 17, 2024
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.

None yet

2 participants