-
Notifications
You must be signed in to change notification settings - Fork 463
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
Split sanity test into functionality and security for test time reduction #4371
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a simple quick checks test to make sure the necessary sanity test markers are applied to a test, and fail if no marker is added?
I anticipate this will be an easy thing to forget, so ideally we have a simple test to notify people of missing markers
I can but there are some tests in sanity directory that do not need any marker at all such sanity/test_canary_integration.py. I can make it potentially make the quick check test skip certain files that do not need to be checked though. |
…nctionality fixture
test/dlc_tests/ec2/test_telemetry.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: Currently, instance type is hard coded in this file. Keep an eye out in pipelines to ensure that this only runs in the telemetry test type job, otherwise you'll have N replicas
non_pr_mainline_tests = ["test_deep_canary_integration"] | ||
|
||
# Navigate through files and look at test files at the top level test/dlc_tests/sanity/ | ||
for item in os.listdir(sanity_test_path): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a simpler way to do this thru pytest api without having to parse thru individual files
GitHub Issue #, if available:
Note:
If merging this PR should also close the associated Issue, please also add that Issue # to the Linked Issues section on the right.
All PR's are checked weekly for staleness. This PR will be closed if not updated in 30 days.
Description
DLC test time reduction by splitting sanity tests into security sanity tests and functionality sanity tests
TEST_TYPE
environment that are initialized as part of the CodeBuild job environment variable.security_sanity
andfunctionality_sanity
test fixtures that will allow each test to run in their respective jobs.telemetry
test fixture that will allow telemetry tests to run in their own job within the MAINLINE context.Tests run
NOTE: By default, docker builds are disabled. In order to build your container, please update dlc_developer_config.toml and specify the framework to build in "build_frameworks"
Confused on how to run tests? Try using the helper utility...
Assuming your remote is called
origin
(you can find out more withgit remote -v
)...python src/prepare_dlc_dev_environment.py -b </path/to/buildspec.yml> -cp origin
python src/prepare_dlc_dev_environment.py -b </path/to/buildspec.yml> -t sanity_tests -cp origin
python src/prepare_dlc_dev_environment.py -rcp origin
NOTE: If you are creating a PR for a new framework version, please ensure success of the standard, rc, and efa sagemaker remote tests by updating the dlc_developer_config.toml file:
Expand
sagemaker_remote_tests = true
sagemaker_efa_tests = true
sagemaker_rc_tests = true
Additionally, please run the sagemaker local tests in at least one revision:
sagemaker_local_tests = true
Formatting
black -l 100
on my code (formatting tool: https://black.readthedocs.io/en/stable/getting_started.html)DLC image/dockerfile
Builds to Execute
Expand
Fill out the template and click the checkbox of the builds you'd like to execute
Note: Replace with <X.Y> with the major.minor framework version (i.e. 2.2) you would like to start.
build_pytorch_training_<X.Y>_sm
build_pytorch_training_<X.Y>_ec2
build_pytorch_inference_<X.Y>_sm
build_pytorch_inference_<X.Y>_ec2
build_pytorch_inference_<X.Y>_graviton
build_tensorflow_training_<X.Y>_sm
build_tensorflow_training_<X.Y>_ec2
build_tensorflow_inference_<X.Y>_sm
build_tensorflow_inference_<X.Y>_ec2
build_tensorflow_inference_<X.Y>_graviton
Additional context
PR Checklist
Expand
NEURON/GRAVITON Testing Checklist
dlc_developer_config.toml
in my PR branch by settingneuron_mode = true
orgraviton_mode = true
Benchmark Testing Checklist
dlc_developer_config.toml
in my PR branch by settingec2_benchmark_tests = true
orsagemaker_benchmark_tests = true
Pytest Marker Checklist
Expand
@pytest.mark.model("<model-type>")
to the new tests which I have added, to specify the Deep Learning model that is used in the test (use"N/A"
if the test doesn't use a model)@pytest.mark.integration("<feature-being-tested>")
to the new tests which I have added, to specify the feature that will be tested@pytest.mark.multinode(<integer-num-nodes>)
to the new tests which I have added, to specify the number of nodes used on a multi-node test@pytest.mark.processor(<"cpu"/"gpu"/"eia"/"neuron">)
to the new tests which I have added, if a test is specifically applicable to only one processor typeBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.