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

Fix tests: flaky and not #54

Merged
merged 10 commits into from
Sep 18, 2020
Merged

Fix tests: flaky and not #54

merged 10 commits into from
Sep 18, 2020

Conversation

atemate
Copy link
Contributor

@atemate atemate commented Sep 17, 2020

Problems:

  1. Workaround for Not Authorized neuro image tags for images with >30 tags platform-registry-api#209. Needed to make CI green to make a release.
  2. Flaky: secret names were hard-coded, therefore parallel builds were conflicting
  3. Flaky: sometimes assert f"git_token={secret}" in result.stdout didn't show up in stdout because this layer was cached and wasn't executed.

Thanks @YevheniiSemendiak and @mariyadavydova for helping debug the problem!

Copy link
Collaborator

@YevheniiSemendiak YevheniiSemendiak left a comment

Choose a reason for hiding this comment

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

Thank you for the investigation and the problem solution! 👍

@@ -13,7 +13,7 @@ jobs:
name: Run tests
strategy:
matrix:
python-version: [3.6, 3.7]
python-version: [3.6], 3.7]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it means, that tests will be executed on 3.6 only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

temp changes, to be reverted

@@ -287,6 +295,7 @@ def test_image_build_env(cli_runner: CLIRunner) -> None:
assert result.returncode == 0, result

secret = str(uuid.uuid4())
print("secret", secret)
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need it for debugging purposes?
mb let's hide it behind

try:
         assert result.returncode == 0, result
except AssertionError as e:
         click.echo(f"secret: {secret}")
         raise

(the same for line 333)

Also, shouldn't the image name in line 165, 271 and 318 be also unique?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

temp changes too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mb let's hide it behind

no need to add logic for debug purposes only, tests should be as simple as possible

@atemate atemate changed the title Fix 401 Not Authorized on neuro image tags test Fix tests: flaky and not Sep 18, 2020
@mariyadavydova mariyadavydova merged commit f2d4a94 into master Sep 18, 2020
@YevheniiSemendiak YevheniiSemendiak deleted the fix-tests-401-unauthorized branch September 25, 2020 12: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.

None yet

3 participants