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 repository declaration order #17101

Merged
merged 1 commit into from May 10, 2021
Merged

Conversation

glefloch
Copy link
Member

This fix change the order of repository declaration in sample test projects.

It should fix release test ci job and tests that fail in PRs.

@quarkus-bot quarkus-bot bot added the area/gradle Gradle label May 10, 2021
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain the rationale here? Do I misunderstand or we will always try to download things instead of using the local cache?

@quarkus-bot
Copy link

quarkus-bot bot commented May 10, 2021

✖ This workflow run has failed but no jobs reported an error. Something weird happened, please check the workflow run page carefully: it might be an issue with the workflow configuration itself.

@glefloch
Copy link
Member Author

Well not exactly. Errors come from the fact that some artifacts are only partially resolved in the local repository (only the pom is present and not the jar). In that case, gradle don't fallback to the next repository to resolve the jar. I opened an issue on gradle for that.

Regarding the resolution, gradle also have a cache for dependencies, dependencies will only download once if they are not already in that cache.

I agree that this is not a durable solution but this avoid to disable those tests.

Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@glefloch
Copy link
Member Author

glefloch commented May 10, 2021

ok, so this won't change. gradle/gradle#17144 (comment)

@gsmet
Copy link
Member

gsmet commented May 10, 2021

@famod looks like there's an issue with the build: Error when evaluating 'strategy' for job 'native-tests'. (Line: 698, Col: 15): Error reading JToken from JsonReader. Path '', line 0, position 0.,(Line: 69

@gsmet gsmet merged commit e410c8f into quarkusio:main May 10, 2021
@quarkus-bot quarkus-bot bot added this to the 2.0 - main milestone May 10, 2021
@famod
Copy link
Member

famod commented May 10, 2021

@gsmet The script is not yet covering this case which was introduced recently: 9bd76ca#diff-307c72aded80fc768e1f20cfa0bed2320c3fe4bb815a6996db89d986c972d845
Will send a fix ASAP, it's rather easy.

@geoand
Copy link
Contributor

geoand commented May 10, 2021

So I broke it, cool :)

The thing is that I had no idea I broke it. Would it be possible to fail the job if something is wrong?

@famod
Copy link
Member

famod commented May 10, 2021

@geoand Thing is that in your PR GIB decided to do a full build due to:

[INFO] gitflow-incremental-builder execution skipped: Changed path matches regex defined by skipIfPathMatches: .github/native-tests.json

and because of that the respective part of the script wasn't run at all (no use filtering any native jobs for a full build).

Nice corner case you hit there! I'll think about making this more fine grained: https://github.com/quarkusio/quarkus/blob/main/pom.xml#L290

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

Successfully merging this pull request may close these issues.

None yet

5 participants