Skip to content

Conversation

lukaszsocha2
Copy link
Contributor

Closes: SDK-2088

@coveralls
Copy link

coveralls commented Apr 25, 2022

Pull Request Test Coverage Report for Build 2226996240

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.588%

Totals Coverage Status
Change from base Build 2096580663: 0.0%
Covered Lines: 3313
Relevant Lines: 3540

💛 - Coveralls

@lukaszsocha2 lukaszsocha2 temporarily deployed to integration-tests April 25, 2022 15:19 Inactive
@lukaszsocha2 lukaszsocha2 temporarily deployed to integration-tests April 25, 2022 15:36 Inactive
@lukaszsocha2 lukaszsocha2 deployed to integration-tests April 26, 2022 12:22 Active
mwwoda
mwwoda previously approved these changes Apr 26, 2022
Copy link
Contributor

@mwwoda mwwoda left a comment

Choose a reason for hiding this comment

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

Approved with a minor comment regarding potential improvements.

from boxsdk.client import Client


def read_jwt_path_from_config(config_path: str):
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you considered supporting .json configuration when running integration tests locally? It may be easier for some developers to download a configuration file and copy it into this particular solution solution, rather than setting an env variable encoded as base64 for the entire machine. For example, our .NET SDK looks for the env variable and, if it is not present, it takes the .json configuration found in the base dir.

@lukaszsocha2 lukaszsocha2 merged commit 518ce25 into main Apr 26, 2022
@lukaszsocha2 lukaszsocha2 deleted the sdk-2088-run-integration-tests-on-ci branch April 26, 2022 13:38
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