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

CI yaml fixed after PR 60 changes, Whitelist regexp fix #61

Merged
merged 1 commit into from
Oct 29, 2021

Conversation

Karm
Copy link
Owner

@Karm Karm commented Oct 28, 2021

No description provided.

@Karm
Copy link
Owner Author

Karm commented Oct 28, 2021

@zakkak

 Error:    DebugSymbolsTest.debugSymbolsQuarkusContainer:256 build-and-run.log log should not contain error or warning lines that are not whitelisted. See /home/runner/work/mandrel-integration-tests/mandrel-integration-tests/ts/testsuite/target/archived-logs/org.graalvm.tests.integration.DebugSymbolsTest/debugSymbolsQuarkusContainer/build-and-run.log and check these offending lines: 
COPY failed: file not found in build context or excluded by .dockerignore: stat target/sources: file does not exist

Seems this is no longer valid?
https://github.com/Karm/mandrel-integration-tests/blob/master/builder-image-apps/quarkus-vertx/src/main/docker/Dockerfile.native#L5

Looking at my old comment....
https://github.com/Karm/mandrel-integration-tests/blob/master/testsuite/src/it/java/org/graalvm/tests/integration/DebugSymbolsTest.java#L259

@zakkak
Copy link
Collaborator

zakkak commented Oct 28, 2021

@zakkak

 Error:    DebugSymbolsTest.debugSymbolsQuarkusContainer:256 build-and-run.log log should not contain error or warning lines that are not whitelisted. See /home/runner/work/mandrel-integration-tests/mandrel-integration-tests/ts/testsuite/target/archived-logs/org.graalvm.tests.integration.DebugSymbolsTest/debugSymbolsQuarkusContainer/build-and-run.log and check these offending lines: 
COPY failed: file not found in build context or excluded by .dockerignore: stat target/sources: file does not exist

Fixed in #62

Seems this is no longer valid? https://github.com/Karm/mandrel-integration-tests/blob/master/builder-image-apps/quarkus-vertx/src/main/docker/Dockerfile.native#L5

No, that seems OK

@Karm Karm force-pushed the fix-after-pr-60 branch 2 times, most recently from 3c45a43 to 21d3942 Compare October 29, 2021 12:57
@Karm
Copy link
Owner Author

Karm commented Oct 29, 2021

@zakkak So, there is still a difference we need to account for, probably by patching before test the same way as compatibility with Quarkus 1.7 was done.

Quarkus 2.2.3.Final + debug symbols + container is just fine.

Quarkus 2.4.0.Final in that scenario is missing a file I am trying to copy in that Dockerfile:

Error:    DebugSymbolsTest.debugSymbolsQuarkusContainer:256 build-and-run.log log should not contain error or warning lines that are not whitelisted. See /home/runner/work/mandrel-integration-tests/mandrel-integration-tests/ts/testsuite/target/archived-logs/org.graalvm.tests.integration.DebugSymbolsTest/debugSymbolsQuarkusContainer/build-and-run.log and check these offending lines: 
COPY failed: file not found in build context or excluded by .dockerignore: stat target/sources: file does not exist

/me running locally to nail it down, but apparently adding Quarkus 2.4.0.Final requires some migration work on TS.

@Karm
Copy link
Owner Author

Karm commented Oct 29, 2021

Q 2.4.0.Final

$ find target/ -name sources
target/quarkus-native-image-source-jar/sources

Q 2.2.3.Final

$ find target/ -name sources
target/sources

@zakkak
Copy link
Collaborator

zakkak commented Oct 29, 2021

@Karm FYI quarkusio/quarkus#20355 is marked for backport to Quarkus 2.2 so eventually it will be the same for both.

I guess our tests still need to cover this case till then though...

@@ -22,12 +23,18 @@ jobs:
strategy:
fail-fast: false
matrix:
quarkus-version: ['2.2.3.Final', '2.4.0.Final']
mandrel-version: ['21.2.0.2-Final', '21.3.0.0-Final']
quarkus-version: ['1.11.7.Final', '2.2.3.Final', '2.4.0.Final']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we still interested in Quarkus 1.11 and Mandrel 20.3?

Copy link
Owner Author

Choose a reason for hiding this comment

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

@zakkak Not for testing Q or M. This is for knowing that TS is still compatible with it. It is too soon to go and purge all if (QUARKUS_VERSION.startsWith("1.")) {... and thus remove the compatibility.

@Karm
Copy link
Owner Author

Karm commented Oct 29, 2021

@zakkak Seems fine now. Ad perf results: I will examine them on a baremetal lab box.

@Karm Karm merged commit bb73e3b into master Oct 29, 2021
@zakkak zakkak deleted the fix-after-pr-60 branch November 2, 2021 15:48
@zakkak zakkak restored the fix-after-pr-60 branch November 2, 2021 15:48
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