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

feat: Cloud Adapter for streaming tests #3835

Merged
merged 4 commits into from
Jul 2, 2021

Conversation

joshgummersall
Copy link
Contributor

  • Moves streaming e2e tree into the testing directory
  • Code formatting
  • Replaces BotFrameworkAdapter with CloudAdapter

#minor

@joshgummersall joshgummersall requested review from stevengum and a team as code owners June 30, 2021 18:40
@coveralls
Copy link

coveralls commented Jun 30, 2021

Pull Request Test Coverage Report for Build 994166019

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.003%) to 84.329%

Totals Coverage Status
Change from base Build 992195184: -0.003%
Covered Lines: 19629
Relevant Lines: 22079

💛 - Coveralls

@joshgummersall
Copy link
Contributor Author

Note: once this lands I'll reconfigure the ADO pipeline(s) to point to the new location.

0,
3
0,
3
)}...${DIRECT_LINE_SECRET.substr(-3)}"`
Copy link
Contributor

@mdrichardson mdrichardson Jul 2, 2021

Choose a reason for hiding this comment

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

Maybe either DIRECT_LINE_SECRET?.substr(-3) or do an argument check (if not already done elsewhere) prior to this step? There's a couple of these calls in this function. Although it looks like it'll send a 500 prior to this step if it's empty. I'm not sure how clear the err.message is for an empty token, though.

Upon further inspection, it might be good to throw in generateDirectlineToken()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to keep this change scoped to just Cloud Adapter. As this is code for CI, I think it's okay to expect the environment is set up correctly.

Copy link
Contributor

@mdrichardson mdrichardson left a comment

Choose a reason for hiding this comment

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

Approved, pending the one unresolved comment I left.

@joshgummersall joshgummersall merged commit e925861 into main Jul 2, 2021
@joshgummersall joshgummersall deleted the jpg/streaming-e2e-cloud-adapter branch July 2, 2021 17:58
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