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

Bszabo/frontend app authoring pipeline testing #21

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

bszabo
Copy link

@bszabo bszabo commented Feb 7, 2025

Tubular script ultimately intended to invoke Datadog synthetic tests in service of a GoCD build pipeline. This early version, however, does not do that. It only executes a single hack-y test intended to be used like a Django waffle switch to control the behavior of the GoCD pipeline responsible for running synthetic tests.

This tubular script expects GoCD configuration to provide keys for Datadog API use as environment variables. It also hard-wires the set of Datadog synthetic tests to be run, and currently has that list be empty.

The one failing unit test was merged into master following an earlier PR and needs to be resolved separately.

Testing on stage: Test in conjunction with the edx-internal PR that creates a new stage-frontend-app-course-authoring pipeline to use of this script. Note that the new pipeline will succeed or fail depending on how the "waffle switch" test has been written. Course authoring builds will be unaffected by whether the new pipeline succeeds or fails.

@bszabo bszabo force-pushed the bszabo/frontend-app-authoring-pipeline-testing branch from 211052d to 3e6e186 Compare February 10, 2025 20:48
@bszabo bszabo force-pushed the bszabo/frontend-app-authoring-pipeline-testing branch from 3e6e186 to 7fadbe3 Compare February 10, 2025 20:59
@bszabo bszabo requested review from dianakhuang and timmc-edx and removed request for timmc-edx February 10, 2025 21:14
Copy link
Member

@dianakhuang dianakhuang left a comment

Choose a reason for hiding this comment

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

Overall, looks great. Just some comments/nitpicks that we can discuss if necessary.

DATADOG_SYNTHETIC_TESTS_API_URL = "https://api.datadoghq.com/api/v1/synthetics/tests"
MAX_ALLOWABLE_TIME_SECS = 600 # 10 minutes

WAFFLE_SWITCH_TEST = SyntheticTest(
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit of a nitpick, but since 'waffle' is the name of the specific library we use in Django to handle toggles, this name is a bit confusing since it doesn't test 'waffle' functionality at all. Maybe something like TESTS_ENABLED? (I am not sure I'm any better at naming 😅 .)

Copy link
Author

Choose a reason for hiding this comment

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

I'll rename to DEPLOYMENT_TESTING_ENABLE_DISABLE_SWITCH

Copy link
Member

Choose a reason for hiding this comment

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

Generally the format we'd use is something like DEPLOYMENT_TESTING_ENABLED. (Sometimes you also see ENABLE_DEPLOYMENT_TESTING but then settings don't sort very well when you're looking at an alphabetized listing!)

Copy link
Author

Choose a reason for hiding this comment

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

I've gone with a compromise, as this macro isn't for the boolean itself, but rather for the source of the boolean

''' Client class to invoke datadog API to run and monitor synthetic tests '''

DATADOG_SYNTHETIC_TESTS_API_URL = "https://api.datadoghq.com/api/v1/synthetics/tests"
MAX_ALLOWABLE_TIME_SECS = 600 # 10 minutes
Copy link
Member

Choose a reason for hiding this comment

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

This is also a bit of a nitpick, but do we want to make this also configurable via the command line interface so that, in theory, different pipelines can use different timeouts?

Copy link
Author

Choose a reason for hiding this comment

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

I'll be including the passing of configuration settings via the CLI in the next pass, and will include timeouts in that. Good call, thanks.

@bszabo bszabo merged commit c18c093 into master Feb 11, 2025
2 of 3 checks passed
@bszabo bszabo deleted the bszabo/frontend-app-authoring-pipeline-testing branch February 11, 2025 18:24
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