Skip to content

e2e tests, CF, agent and guest #1173

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

e2e tests, CF, agent and guest #1173

wants to merge 14 commits into from

Conversation

giavac
Copy link
Collaborator

@giavac giavac commented Feb 13, 2025

Description

Adds e2e tests for Call Fabric scenarios like Agent and Customer (guest token) joining the same cXML conference.

  • One creates a cXML application with a static script
  • One creates a cXML application with an external URL
  • Another uses a static cXML script to connect a customer (guest token) to a stream (this is currently commented out)

It requires two new ENV vars:

  • EXTERNAL_URL_FOR_CXML, pointing to the external URL to be used in the second scenario.
  • CXML_STREAM_URL, pointing to the desired stream URL.

Type of change

  • Internal refactoring
  • Bug fix (bugfix - non-breaking)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Code snippets

In case of new feature or breaking changes, please include code snippets.

Copy link

changeset-bot bot commented Feb 13, 2025

⚠️ No Changeset found

Latest commit: d7a70c9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@giavac giavac changed the title WIP - e2e tests, CF, agent and guest e2e tests, CF, agent and guest Feb 25, 2025
@iAmmar7
Copy link
Member

iAmmar7 commented Feb 26, 2025

I remember one more thing; since this is a CF e2e test, do you mind adding this to the Playwright "callfabric" project here? Currently, it is probably in the "default" project.

@giavac giavac requested a review from iAmmar7 February 26, 2025 18:34
iAmmar7
iAmmar7 previously approved these changes Feb 27, 2025
Copy link
Member

@iAmmar7 iAmmar7 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for these amazing tests. 🚀

Let's update the ENV for the production.

@@ -134,11 +134,13 @@ test.describe(agent_customer_static_scripts_desc, () => {

const agent_customer_external_url_desc = 'CallFabric Agent/Customer interaction, cXML with external URL'
test.describe(agent_customer_external_url_desc, () => {
const external_url_for_cxml = process.env.EXTERNAL_URL_FOR_CXML || 'https://us-central1-video-load-testing-with-gcf.cloudfunctions.net/gcf-external-url'
Copy link
Contributor

Choose a reason for hiding this comment

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

could this one var be loaded from a GHA secret, separate from the current block? Then we just need to add one secret, not update all of them or hardcode this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, that would be fine for me. If we add EXTERNAL_URL_FOR_CXML as a variable, I guess it will be accessible in the same way, with process.env. EXTERNAL_URL_FOR_CXML?

Copy link
Contributor

Choose a reason for hiding this comment

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

the env needs to be hydrated of sorts by the secret, I think like #1188

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, I've cherry-picked that change.

@giavac giavac requested a review from ryanwi March 7, 2025 08:17
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