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

chore(test): remove some incompatible in compose test #2623

Merged

Conversation

suyanhanx
Copy link
Contributor

@suyanhanx suyanhanx commented Nov 2, 2023

close #2622

@suyanhanx suyanhanx marked this pull request as ready for review November 2, 2023 10:14
@suyanhanx suyanhanx force-pushed the remove-incompatible-in-compose-test branch from 297ed1d to d42b00b Compare November 2, 2023 10:15
@suyanhanx suyanhanx force-pushed the remove-incompatible-in-compose-test branch 2 times, most recently from cc0bc32 to 211848b Compare November 2, 2023 11:08
Copy link
Contributor

@yuchanns yuchanns left a comment

Choose a reason for hiding this comment

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

LGTM

@AkihiroSuda AkihiroSuda added this to the v1.7.0 milestone Nov 2, 2023
@AkihiroSuda AkihiroSuda added area/ci e.g., CI failure area/compose labels Nov 2, 2023
@@ -60,8 +57,12 @@ services:
base.ComposeCmd("-p", projectName, "-f", compFull.YAMLFullPath(), "up", "-d").AssertOK()
defer base.ComposeCmd("-p", projectName, "-f", compFull.YAMLFullPath(), "down", "--remove-orphans").AssertOK()

base.ComposeCmd("-p", projectName, "-f", compOrphan.YAMLFullPath(), "down", "-v").AssertCombinedOutContains("is in use")

// The error output is different with docker
Copy link
Member

Choose a reason for hiding this comment

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

Can we fix this?

Copy link
Member

Choose a reason for hiding this comment

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

Or just check "in use"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or just check "in use"

It looks okay. But it may not be clear enough as to the meaning of whether or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

base.ComposeCmdWithHelper(unbuffer, "-f", comp.YAMLFullPath(), "exec", "-t=false", "svc0", "stty").AssertFail() // `-i`
base.ComposeCmdWithHelper(unbuffer, "-f", comp.YAMLFullPath(), "exec", "-i=false", "-t=false", "svc0", "stty").AssertFail()

// The error output is different with docker
Copy link
Member

Choose a reason for hiding this comment

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

Can we fix this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. They even don't have the same exit code. We might have to deal with this one separately.

Copy link
Member

Choose a reason for hiding this comment

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

Why can't we make the exit code same?

Copy link
Member

Choose a reason for hiding this comment

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

If it is difficult for now it should be just marked as DockerIncompatible and we can revisit it later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change has been reverted.

@AkihiroSuda AkihiroSuda removed this from the v1.7.0 milestone Nov 2, 2023
@suyanhanx suyanhanx force-pushed the remove-incompatible-in-compose-test branch from 211848b to ccace18 Compare November 4, 2023 11:55
Signed-off-by: Han Xu <suyanhanx@gmail.com>
@suyanhanx suyanhanx force-pushed the remove-incompatible-in-compose-test branch from ccace18 to d9d684b Compare November 5, 2023 09:11
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda added this to the v1.7.1 (tentative) milestone Nov 14, 2023
@AkihiroSuda AkihiroSuda merged commit 171d761 into containerd:main Nov 14, 2023
22 checks passed
@suyanhanx suyanhanx deleted the remove-incompatible-in-compose-test branch November 14, 2023 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci e.g., CI failure area/compose
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compose tests: remove DockerIncompatible()
3 participants