Skip to content

only publish images if image tests run#11123

Merged
jakelishman merged 9 commits into
Qiskit:mainfrom
AngeloDanducci:ad-10968
Oct 27, 2023
Merged

only publish images if image tests run#11123
jakelishman merged 9 commits into
Qiskit:mainfrom
AngeloDanducci:ad-10968

Conversation

@AngeloDanducci
Copy link
Copy Markdown
Contributor

Summary

Fixes #10968

Details and comments

Need to open PR to verify CI tests work correctly, fixes the flake of too early a failure causing image publishing to fail

@AngeloDanducci AngeloDanducci requested a review from a team as a code owner October 26, 2023 18:25
@qiskit-bot
Copy link
Copy Markdown
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@AngeloDanducci AngeloDanducci marked this pull request as draft October 26, 2023 19:41
@AngeloDanducci
Copy link
Copy Markdown
Contributor Author

Will re convert after CI has verified correctly I've removed temporary test changes.

@AngeloDanducci AngeloDanducci marked this pull request as ready for review October 27, 2023 06:07
@qiskit-bot
Copy link
Copy Markdown
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@coveralls
Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 6663551130

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 7 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.004%) to 86.935%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 93.76%
qiskit/extensions/quantum_initializer/squ.py 2 84.15%
crates/qasm2/src/lex.rs 4 92.42%
Totals Coverage Status
Change from base Build 6661399441: 0.004%
Covered Lines: 73966
Relevant Lines: 85082

💛 - Coveralls

@1ucian0 1ucian0 added type: qa Issues and PRs that relate to testing and code quality Changelog: None Do not include in the GitHub Release changelog. labels Oct 27, 2023
1ucian0
1ucian0 previously approved these changes Oct 27, 2023
Copy link
Copy Markdown
Member

@1ucian0 1ucian0 left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the fix!

Copy link
Copy Markdown
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Thanks for this - this looks like the kind of solution I was envisaging!

Minor: please could we tidy up the trailing whitespace?

Comment thread .azure/test-linux.yml Outdated
# This variable should become redundant on release of jupyter-core 6.
JUPYTER_PLATFORM_DIRS: 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change

Comment thread .azure/test-linux.yml Outdated
archiveFile: '$(Build.ArtifactStagingDirectory)/visual_test_failures.tar.gz'
verbose: true
condition: failed()
condition: and(failed(), eq(variables.HAVE_VISUAL_TESTS_RUN, 'true'))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
condition: and(failed(), eq(variables.HAVE_VISUAL_TESTS_RUN, 'true'))
condition: and(failed(), eq(variables.HAVE_VISUAL_TESTS_RUN, 'true'))

Comment thread .azure/test-linux.yml Outdated
archiveFile: '$(Build.ArtifactStagingDirectory)/circuit_results.tar.gz'
verbose: true
condition: failed()
condition: and(failed(), eq(variables.HAVE_VISUAL_TESTS_RUN, 'true'))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
condition: and(failed(), eq(variables.HAVE_VISUAL_TESTS_RUN, 'true'))
condition: and(failed(), eq(variables.HAVE_VISUAL_TESTS_RUN, 'true'))

Comment thread .azure/test-linux.yml Outdated
archiveFile: '$(Build.ArtifactStagingDirectory)/graph_results.tar.gz'
verbose: true
condition: failed()
condition: and(failed(), eq(variables.HAVE_VISUAL_TESTS_RUN, 'true'))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
condition: and(failed(), eq(variables.HAVE_VISUAL_TESTS_RUN, 'true'))
condition: and(failed(), eq(variables.HAVE_VISUAL_TESTS_RUN, 'true'))

Comment thread .azure/test-linux.yml Outdated
Parallel: true
ParallelCount: 8
condition: failed()
condition: and(failed(), eq(variables.HAVE_VISUAL_TESTS_RUN, 'true'))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
condition: and(failed(), eq(variables.HAVE_VISUAL_TESTS_RUN, 'true'))
condition: and(failed(), eq(variables.HAVE_VISUAL_TESTS_RUN, 'true'))

Copy link
Copy Markdown
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

Thanks!

@jakelishman jakelishman enabled auto-merge October 27, 2023 18:28
@jakelishman jakelishman added the stable backport potential Make Mergify open a backport PR to the most recent stable branch on merge. label Oct 27, 2023
@jakelishman jakelishman added this pull request to the merge queue Oct 27, 2023
Merged via the queue into Qiskit:main with commit 905cfde Oct 27, 2023
mergify Bot pushed a commit that referenced this pull request Oct 27, 2023
* only publish images if image tests run

* fix indent in bash command

* fix condition syntax

* set value in the image test

* test runner stopping correctly on fail

* revert purposeful failure

* tidy up trailing whitespace

(cherry picked from commit 905cfde)
github-merge-queue Bot pushed a commit that referenced this pull request Oct 28, 2023
* only publish images if image tests run

* fix indent in bash command

* fix condition syntax

* set value in the image test

* test runner stopping correctly on fail

* revert purposeful failure

* tidy up trailing whitespace

(cherry picked from commit 905cfde)

Co-authored-by: Angelo Danducci <angelo.danducci.ii@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: None Do not include in the GitHub Release changelog. stable backport potential Make Mergify open a backport PR to the most recent stable branch on merge. type: qa Issues and PRs that relate to testing and code quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sometimes visual testing fail with no such file or directory

5 participants