Skip to content

test: unskip nova tests#5901

Merged
lucasjia-aws merged 3 commits into
aws:masterfrom
lucasjia-aws:unskip-nova-tests
May 29, 2026
Merged

test: unskip nova tests#5901
lucasjia-aws merged 3 commits into
aws:masterfrom
lucasjia-aws:unskip-nova-tests

Conversation

@lucasjia-aws
Copy link
Copy Markdown
Collaborator

@lucasjia-aws lucasjia-aws commented May 28, 2026

Summary

Enable Nova model integration tests to run in the dedicated us-east-1 test account (784379639078). This PR migrates the Nova tests from being skipped to being selectable via a new us_east_1 pytest marker, allowing them to run in the new account's CI infrastructure set up in #5900.

Changes

  • New us_east_1 pytest marker (tox.ini): Registered a new marker to identify tests that require us-east-1 account credentials.
  • Unskip Nova tests: Replaced @pytest.mark.skip with @pytest.mark.us_east_1 on all Nova-related integration tests.
  • Update resource references: Updated S3 paths, ARNs, and account IDs from the old account (729646638167) to the new dedicated test account.
  • Extend timeouts: Increased max_wait_time to 3 hours for SFT/RLVR trainer tests and benchmark evaluator test, as Nova training jobs take >1 hour to complete.

Tests affected

Test File Markers
test_sft_trainer_nova_workflow test_sft_trainer_integration.py gpu_intensive, us_east_1
test_rlvr_trainer_nova_workflow test_rlvr_trainer_integration.py gpu_intensive, us_east_1
test_benchmark_evaluation_nova_model test_benchmark_evaluator.py gpu_intensive, us_east_1
test_create_dataset_from_s3_nova_* (4 tests) test_dataset.py serial, us_east_1

How it works

  • PR checks (us-east-1 CodeBuild): Runs -m "us_east_1 and not serial" and -m "us_east_1 and serial" via the integ-tests-us-east-1 job added in test: add a job to invoke code build project in us-east-1 #5900.
  • Scheduled GPU tests: The gpu_intensive marked tests (SFT, RLVR, benchmark) will run in the scheduled gpu-integ-tests-us-east-1 job.
  • Existing us-west-2 tests: Unaffected. The us-west-2 integ test job should add -m "not us_east_1" to exclude these tests.

Testing

All 7 tests validated via PR check runs

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are we running these tests in a different account? E.g. 784379639078 instead of 729646638167?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

yes, @mujtaba1747 set that up earlier

@lucasjia-aws lucasjia-aws merged commit 24e6ef0 into aws:master May 29, 2026
16 of 21 checks passed
"""Test creating RLVR dataset from S3 URI."""
"""Test creating Nova DPO dataset from S3 URI."""
s3_uri = f"s3://{test_bucket}/test_datasets/Nova/nova_dpo_train.jsonl"
dataset = DataSet.create(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There's no cleanup for DataSet.create()?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There is cleanup — the cleanup_list fixture handles it. Each test appends the created dataset to cleanup_list, and the fixture's teardown calls resource.delete() on all tracked resources (see conftest.py line ~108-113).

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