Skip to content

Conversation

@mccoyp
Copy link
Member

@mccoyp mccoyp commented Jan 14, 2022

Description

Context: test IDs, which are used by the proxy to determine recording locations, are currently determined based on the value of the pytest-controlled PYTEST_CURRENT_TEST environment variable. This variable's value isn't consistent across environments though -- the path to the test file is sometimes relative to the repository root, and sometimes not. See #22387 (comment) for more details.

To adjust to this variation in the environment variable, this PR adds logic to determine the repo-root relative path to the current test. This ensures that the test ID is always consistent.

Note: AzureRecordedTestCase's create_random_name method also relies on the value of PYTEST_CURRENT_TEST. The inconsistency of this variable could result in different pseudo-random resource names, which could cause test failures in playback. Fixing this is tracked by #22512.

@scbedd for notifs

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • N/A Pull request includes test coverage for the included changes.
    • An already-migrated test suite is used to verify these changes

@mccoyp
Copy link
Member Author

mccoyp commented Jan 14, 2022

/azp run python - tables

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mccoyp
Copy link
Member Author

mccoyp commented Jan 14, 2022

/azp run python - tables

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mccoyp mccoyp marked this pull request as ready for review January 14, 2022 19:32
@mccoyp mccoyp requested review from scbedd and tjprescott January 14, 2022 19:45
# walk up to the repo root by looking for "sdk" directory or root of file system
path_components = []
head, tail = os.path.split(full_path_to_test)
while tail != "sdk" and tail != "":
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you have any other stop cases here. Famous last words 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Writing a while loop always terrifies me, but I'm also pretty sure this should be good. Knocking furiously on wood

Copy link
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

If it unblocks me, I'm happy with it :)

@mccoyp mccoyp merged commit 8f3a4c7 into Azure:main Jan 14, 2022
@mccoyp mccoyp deleted the proxy-testpath branch January 14, 2022 21:05
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jan 18, 2022
…into add_back_error_message

* 'main' of https://github.com/Azure/azure-sdk-for-python:
  switch error str tests to in (Azure#22016)
  drop py27 support (Azure#22531)
  Update TextAnalytics to enable live testing in sovereign clouds for multiple services (Azure#22461)
  fix: body is too long when create github release (Azure#22522)
  [AutoRelease] t2-labservices-2022-01-10-05622 (Azure#22401)
  [ACR] Change to support python3.6 or above only (Azure#22325)
  Sync eng/common directory with azure-sdk-tools for PR 2554 (Azure#22515)
  [purview catalog] regen with guids rest name fix (Azure#22495)
  Migrate EG tests to test proxy (Azure#21772)
  [Test Proxy] Normalize paths in test IDs (Azure#22508)
  Add packages to $PackageExclusions which do not build properly in Docs CI (Azure#22493)
  Update Manifest Publishing (Azure#22476)
  Issue Azure#9324 by DaisyCisneros (Azure#21824)
rakshith91 pushed a commit to rakshith91/azure-sdk-for-python that referenced this pull request Apr 10, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-python that referenced this pull request Mar 15, 2023
Merge release sentinel 2023 03 01 preview (Azure#22983)

* Adds base for updating Microsoft.SecurityInsights from version preview/2023-02-01-preview to version 2023-03-01-preview

* Updates readme

* Updates API version in new specs and examples

* Update description of AAD (Azure#22508)

* Update description of AAD

AAD should be AADIP to prevent mistakes from customer

* Add custom word AADIP 

AADIP = Azure Active Directory Identity Protection

* Remove bing phishing feed msti connector (Azure#22706)

* remove bing phishing feed from msti connector

* revert to release-Sentinel-2023-03-01-preview

* fixes

* fixed validation issues

* renamed example file

* added response status codes for examples

* Automation rules - 2023-03-01-preview - add entity trigger API (Azure#22523)

* adjust version order

* Fix cross-version breaking changes.

Fix cross-version breaking changes.

* Pull in metadata changes from stable version.

* Update Metadata.json

* Update Incidents.json

add readonly flag for providerIncidentId.

* Update readme.md

GuidUsage AutoRest-Linter suppression.

* Update readme.md

suppress GuidUsage

* Update readme.md

Correct suppression.

* Update readme.md

Fix suppression.

---------

Co-authored-by: shschwar <[email protected]>
Co-authored-by: aviyerMSFT <[email protected]>
Co-authored-by: loriatarms <[email protected]>
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.

3 participants