Skip to content

Conversation

@owais
Copy link
Contributor

@owais owais commented Jun 3, 2021

Description

Run tests on Windows

Type of change

Please delete options that are not relevant.

  • Tooling/Dev enchancement
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Updated tests

Does This PR Require a Core Repo Change?

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@owais owais force-pushed the windows-ci branch 17 times, most recently from 386e778 to 51e7caf Compare June 4, 2021 10:02
@owais owais added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Aug 24, 2021
@owais owais force-pushed the windows-ci branch 12 times, most recently from 9e679af to f00d06c Compare August 25, 2021 23:48
Copy link
Member

@aabmass aabmass left a comment

Choose a reason for hiding this comment

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

LGTM

self.assertIsNone(client.parent)
self.assertEqual(client.kind, SpanKind.CLIENT)
self.assert_span_has_attributes(
self.assertSpanHasAttributes(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you split the renaming out of this PR? It would make reviews a bit cleaner

Copy link
Contributor Author

Choose a reason for hiding this comment

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

open-telemetry/opentelemetry-python#2084
#641

Easiest thing to do right now for me is to merge the above PRs and then rebase this one to shrink the size of the PR. Please review.


; opentelemetry-instrumentation-sqlalchemy
py3{6,7,8,9}-test-instrumentation-sqlalchemy{11,14}
py3{6,7}-test-instrumentation-sqlalchemy{11}
Copy link
Contributor

Choose a reason for hiding this comment

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

@ocelotl was this issue filed?

| if .[0].benchmarks == null then null else .[0] end'
**/**/tests/*${{ matrix.package }}*-benchmark.json > output.json
- name: Report on benchmark results
# TODO: fix and enable on windows
Copy link
Contributor

Choose a reason for hiding this comment

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

Please file an issue to track 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.

- name: run tox
run: tox -f ${{ matrix.python-version }}-${{ matrix.package }} -- --benchmark-json=${{ env.RUN_MATRIX_COMBINATION }}-benchmark.json
- name: Find and merge benchmarks
# TODO: fix and enable on windows
Copy link
Contributor

Choose a reason for hiding this comment

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

Please file an issue to track 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.

@owais owais force-pushed the windows-ci branch 2 times, most recently from 8a2955d to e48eb18 Compare August 30, 2021 22:34
@owais owais force-pushed the windows-ci branch 2 times, most recently from 013adb2 to ceac768 Compare August 30, 2021 23:18
@owais
Copy link
Contributor Author

owais commented Aug 30, 2021

I'm running into some more issues with pypy on windows. Looking into it. Please DO NOT merge even if all jobs pass.

@owais owais force-pushed the windows-ci branch 3 times, most recently from b56e6c8 to c881bf5 Compare August 30, 2021 23:40
@owais
Copy link
Contributor Author

owais commented Aug 31, 2021

Turns out cryptography already published prebuilt binary wheels for windows+pypy+amd64. Using x64 Python fixes the issue and doesn't require us to build it from source.

@owais owais requested review from aabmass and codeboten August 31, 2021 01:02
@owais owais mentioned this pull request Aug 31, 2021
8 tasks
@ocelotl
Copy link
Contributor

ocelotl commented Sep 1, 2021

I'm running into some more issues with pypy on windows. Looking into it. Please DO NOT merge even if all jobs pass.

@owais Please mark any PR that is not to be merged as draft to avoid accidents ✌️

@owais owais marked this pull request as draft September 1, 2021 12:35
Copy link
Contributor

@codeboten codeboten 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 splitting out the PR, makes this review much easier 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants