# GitHub Copilot Instructions for Scylla Cluster Tests

This file provides instructions for GitHub Copilot when working with the Scylla Cluster Tests (SCT) repository.

## Commit Message Format

All commits must follow the [Conventional Commits](https://www.conventionalcommits.org/) format with the following structure:

```
<type>(<scope>): <subject>

<body>

[reference]
```

### Commit Message Rules

**Type** (required): Must be one of:
- `ci` - Changes to CI configuration files and scripts
- `docs` - Documentation only changes
- `feature` - A new feature
- `fix` - A bug fix
- `improvement` - An improvement to existing functionality
- `perf` - A code change that improves performance
- `refactor` - A code change that neither fixes a bug nor adds a feature
- `revert` - Reverts a previous commit
- `style` - Changes that do not affect the meaning of the code (formatting, etc.)
- `test` - Adding missing tests or correcting existing tests
- `unit-test` - Specific unit test changes
- `build` - Changes that affect the build system or external dependencies
- `chore` - Other changes that don't modify src or test files

**Scope** (required):
- Minimum 3 characters
- Should describe the area of the codebase affected (e.g., `aws-provision`, `azure`, `docker`, `nemesis`, `config`)
- Examples: `cluster-aws`, `stress-tools`, `monitoring`, `k8s-operator`

**Subject** (required):
- Minimum 10 characters, maximum 120 characters
- Must not end with a period
- Should be a concise description of the change

**Header** (type + scope + subject):
- Maximum 100 characters total

**Body** (required):
- Minimum 30 characters
- Maximum 120 characters per line
- Must be separated from subject by a blank line
- Should explain what and why, not how
- Can include multiple paragraphs

**Reference** (optional):
- Format: `[issue-number]` or `[PR-number]`
- Example: `[#1234]`

### Example Commit Messages

Good commit message:
```
fix(aws-provision): properly print count of nodes in provisioning phase

Fixed a bug where the node count was not being displayed correctly
during the AWS provisioning phase. This improves debugging experience.

[#5678]
```

Another example:
```
feature(nemesis): add new disk failure simulation

Implemented a new nemesis operation to simulate disk failures in the
cluster. This helps test database resilience under storage issues.
```

## Pre-commit Requirements

**CRITICAL**: Before every commit, you MUST run pre-commit checks to ensure code quality:

```bash
uv run sct.py pre-commit
```

The pre-commit hooks will:
- Fix trailing whitespace and end-of-file issues
- Validate YAML files
- Check for large files and sensitive data
- Run `autopep8` for Python code formatting
- Run `ruff` linter with auto-fix
- Update configuration documentation
- Validate commit messages with `commitlint`
- Sort dependencies in `pyproject.toml`

**Never commit code that fails pre-commit checks.** Fix all issues before committing.

### Pre-commit Configuration

The repository uses `.pre-commit-config.yaml` for configuration. Key hooks include:
- **autopep8**: Automatically formats Python code
- **ruff**: Fast Python linter with auto-fix capabilities
- **commitlint**: Validates commit message format
- Custom hooks for updating documentation and generating nemesis configurations

### Auto-Generated Files

**IMPORTANT**: Do not commit auto-generated files to your PR. The following directories contain auto-generated files:

- `configurations/nemesis/` - Nemesis configuration files auto-generated by pre-commit hooks
- `jenkins-pipelines/oss/nemesis/` - Jenkins pipeline files auto-generated by pre-commit hooks

**What to do if these files appear in your PR:**

1. **Do NOT commit them** - These files are generated by pre-commit hooks and should not be part of your PR
2. **Remove them from staging**: Use `git rm` to unstage and remove these files
3. **Notify the requester**: If these files were generated, it may indicate that another PR failed to generate them properly. Mention this in your PR comments.

**Example of removing auto-generated files:**
```bash
# Remove nemesis configuration files
git rm configurations/nemesis/YourGeneratedFile.yaml

# Remove jenkins pipeline files
git rm jenkins-pipelines/oss/nemesis/longevity-5gb-1h-YourGeneratedFile-*.jenkinsfile
```

**Why these files are auto-generated:**
- Pre-commit hooks automatically generate nemesis configurations and Jenkins pipelines
- These files are managed by the build system and should be generated during CI/CD
- Including them in PRs causes merge conflicts and clutters the change history

## Test Format and Location

### Test Requirements

All tests MUST:
- Be written in **pytest** format
- Be placed in the `./unit_tests/` directory
- Follow the naming convention: `test_*.py`
- Use appropriate pytest markers for categorization

### Test Structure

```python
import pytest

class TestFeatureName:
    """Test suite for feature X."""

    def test_specific_behavior(self):
        """Test that specific behavior works correctly."""
        # Arrange
        expected = "value"

        # Act
        result = function_to_test()

        # Assert
        assert result == expected
```

### Pytest Markers

Use these markers to categorize your tests:

```python
@pytest.mark.integration  # For integration tests
@pytest.mark.provisioning  # For tests requiring resource provisioning
@pytest.mark.need_network  # For tests requiring network access
```

### Test Organization

- `unit_tests/` - Main unit test directory
  - `lib/` - Tests for library utilities
  - `nemesis/` - Tests for nemesis operations
  - `provisioner/` - Tests for provisioning logic
  - `rest/` - Tests for REST API clients
  - `test_*.py` - Individual test modules

### Running Tests

```bash
# Run all unit tests
uv run sct.py unit-tests

# Run specific test file
uv run sct.py unit-tests -t test_config.py

# Run integration tests
uv run sct.py integration-tests
```

### Testing Workflow

When you edit or create a file matching `unit_tests/test_*.py`, always suggest running:
```bash
uv run python -m pytest <file> -x -q --no-header --tb=short -n0
```
and wait for the results before proceeding to the next change.

## Backend-Specific Provision Tests

### When to Add Provision Test Labels

When you modify code that touches backend-specific provisioning logic, you MUST ensure that PRs are labeled with appropriate provision test labels to trigger backend-specific testing.

### Backend Files Mapping

If you modify any of these files, add the corresponding provision test label:

**AWS Backend** (`test-provision-aws` label):
- `sdcm/cluster_aws.py` - AWS cluster implementation
- `sdcm/provision/aws/*` - AWS-specific provisioning
- `sdcm/utils/aws_utils.py` - AWS utilities

**GCE Backend** (`test-provision-gce` label):
- `sdcm/cluster_gce.py` - GCE cluster implementation
- `sdcm/provision/gce/*` - GCE-specific provisioning (if exists)
- `sdcm/utils/gce_utils.py` - GCE utilities

**Azure Backend** (`test-provision-azure` label):
- `sdcm/cluster_azure.py` - Azure cluster implementation
- `sdcm/provision/azure/*` - Azure-specific provisioning
- `sdcm/utils/azure_utils.py` - Azure utilities

**Docker Backend** (`test-provision-docker` label):
- `sdcm/cluster_docker.py` - Docker cluster implementation
- `sdcm/utils/docker_utils.py` - Docker utilities

**Kubernetes Backends** (`test-provision-k8s-eks` label):
- `sdcm/cluster_k8s/*` - Kubernetes cluster implementations
- `sdcm/utils/k8s/*` - Kubernetes utilities

**OCI Backend** (`test-provision-oci` label):
- `sdcm/cluster_oci.py` - OCI cluster implementation
- `sdcm/provision/oci/*` - OCI-specific provisioning

### How to Add Labels

In your PR description, mention the backends affected:

```markdown
## Backends Affected
- [x] AWS - Modified cluster_aws.py
- [ ] GCE
- [ ] Azure
- [ ] Docker
```

Or explicitly request labels:
```markdown
## Required Labels
- `test-provision-aws`
- `test-provision-docker`
```

## Manual Testing Notes in PRs

For every PR, you MUST include a section describing manual tests that Copilot couldn't perform automatically. This helps reviewers understand what additional validation is needed.

### Required Manual Testing Section

Add this section to your PR description:

```markdown
## Manual Testing Required

### What Copilot Couldn't Test

- [ ] **End-to-end cluster provisioning**: Requires AWS account and takes 30+ minutes
  - Test: `hydra run-test longevity_test.LongevityTest.test_custom_time --backend aws`
  - Expected: Cluster provisions successfully and test runs

- [ ] **Nemesis operations on real cluster**: Requires actual cluster infrastructure
  - Test: Run with `nemesis_selector: 'DecommissionMonkey'`
  - Expected: Node decommission completes without data loss

- [ ] **Performance impact**: Requires production-like load
  - Test: Performance regression test with cassandra-stress
  - Expected: No significant performance degradation (< 5%)

- [ ] **Multi-region functionality**: Requires multi-region AWS setup
  - Test: Configure `regions: ['us-east-1', 'us-west-2']`
  - Expected: Cross-region replication works correctly

### Suggested Manual Test Commands

\`\`\`bash
# Test 1: Basic provisioning
export SCT_SCYLLA_VERSION=5.2.1
hydra run-test longevity_test.LongevityTest.test_custom_time --backend docker

# Test 2: With specific nemesis
hydra run-test longevity_test.LongevityTest.test_custom_time \
  --backend aws \
  --config test-cases/your-test.yaml
\`\`\`

### Environment Requirements

- AWS credentials configured
- Docker installed and running
- Sufficient system resources (16GB RAM, 50GB disk)
```

### Examples of What to Include

**Infrastructure Changes:**
- "Verify cluster provisioning on AWS with new instance types"
- "Test network configuration changes with IPv6 setup"
- "Validate security group modifications don't block required ports"

**Nemesis Changes:**
- "Run longevity test with new nemesis for at least 1 hour"
- "Verify cluster recovers correctly after nemesis operation"
- "Check that data remains consistent after disruption"

**Performance Changes:**
- "Run performance regression tests and compare with baseline"
- "Measure impact on throughput and latency under load"
- "Profile memory usage with production-size dataset"

**Monitoring/Reporting Changes:**
- "Verify Grafana dashboards display new metrics"
- "Check that Argus reports contain new data points"
- "Validate email reports format correctly"

## Commit History and Squashing

### Preferred Approach: Single Logical Commits

When working on PRs, the **preferred approach** is to create single, logical commits rather than multiple incremental commits. This makes the git history cleaner and easier to review.

**Best Practice:**
- Combine all related changes (code, tests, documentation) into a single comprehensive commit
- If you need to create multiple commits during development, squash them before the PR is merged
- Use `git rebase -i` to squash commits locally, then provide the squashed commit hash for manual force push

**When Squashing is Needed:**
If you've already pushed multiple commits and need to squash them:

1. Use `git rebase -i <base_commit>` to squash commits locally
2. Create a well-formatted commit message that explains all changes
3. Provide the squashed commit hash to the reviewer
4. The reviewer will manually force push the squashed commit

**Example workflow:**
```bash
# Squash last 4 commits
git rebase -i HEAD~4

# Mark commits as 'squash' or 's' except the first one
# Edit the combined commit message
# Provide the resulting commit hash for force push
```

This ensures:
- Clean, linear git history
- Each commit is self-contained and tests pass
- Easier to understand changes and revert if needed
- Better commit messages that explain the full context

## Additional Guidelines

### Code Quality Standards

1. **Always run pre-commit before committing** - This is non-negotiable
2. **Write tests for new functionality** - Place them in `unit_tests/`
3. **Update documentation** - Modify relevant files in `docs/` if needed
4. **Follow existing patterns** - Look at similar code in the repository
5. **Keep changes minimal** - Only modify what's necessary

### Configuration Changes

When adding new configuration options:
1. Update `sdcm/sct_config.py` with the new parameter
2. Add documentation to the parameter definition
3. **REQUIRED**: Add default values in `defaults/test_default.yaml` or relevant backend-specific files in `defaults/` (e.g., `aws_config.yaml`, `gce_config.yaml`, etc.)
   - Every configuration option in `sct_config.py` MUST have a corresponding default value
   - This ensures consistent behavior and prevents `None` values when parameters are not explicitly set
4. The pre-commit hook will automatically update `docs/configuration_options.md`

### Testing Strategy

1. **Unit tests** - Test individual functions and classes in isolation
2. **Integration tests** - Test component interactions (mark with `@pytest.mark.integration`)
3. **Provisioning tests** - Test infrastructure provisioning (mark with `@pytest.mark.provisioning`)
4. **Manual tests** - Document in PR for complex scenarios

### Common Test Patterns

```python
# Testing with fixtures
def test_with_fixture(tmpdir):
    """Test using pytest's tmpdir fixture."""
    test_file = tmpdir.join("test.yaml")
    test_file.write("key: value")
    assert test_file.read() == "key: value"

# Testing exceptions
def test_raises_exception():
    """Test that function raises expected exception."""
    with pytest.raises(ValueError, match="Invalid input"):
        function_that_should_fail("bad_input")

# Parametrized tests
@pytest.mark.parametrize("input,expected", [
    ("value1", "result1"),
    ("value2", "result2"),
])
def test_multiple_cases(input, expected):
    """Test multiple cases efficiently."""
    assert function(input) == expected
```

## Code Review: Inheritance & Override Safety

**CRITICAL — SCT uses deep class hierarchies with polymorphic overrides.**

When reviewing a PR that changes a method signature on a base or parent class:

1. **Find ALL overrides** — search the codebase for `def <method_name>` in subclasses.
   Key polymorphic methods with multiple overrides:
   - `_create_instances` (5+ overrides: `cluster_aws.py`, `cluster_gce.py`, `cluster_azure.py`, `cluster_oci.py`, `cluster_k8s/eks.py`)
   - `add_nodes` (18+ overrides across all backends, K8S, Docker, test stubs)
   - `_create_on_demand_instances`, `_create_spot_instances` (AWS-specific)
   - `wait_for_init`, `destroy` (all backend files)

2. **Verify signature compatibility** — every override must accept the new parameter(s),
   either explicitly or via `**kwargs`. If an override calls `super()`, the new parameter
   must be forwarded.

3. **Check callers** — if callers now pass the new parameter, every implementation in the
   dispatch chain must handle it.

4. **Check test stubs** — `unit_tests/dummy_remote.py`, `unit_tests/test_cluster.py`, and
   `unit_tests/test_scylla_yaml_builders.py` contain stub overrides that must also match.

**Why**: PR #13445 added `ami_id` to `AWSCluster._create_instances` but missed
`MonitorSetEKS._create_instances` (an override in `cluster_k8s/eks.py`), causing
a `TypeError` at runtime. This was not caught in multiple review rounds (PR #14113 fix).

**Rule of thumb**: If you see a method signature change on any class that has the word
`Cluster`, `Node`, `Set`, or `Monitor` in its name — assume there are overrides you
haven't seen in the diff, and search for them.

For comprehensive code review guidance, see `skills/code-review/SKILL.md`.

## Resources

- [Repository README](../README.md) - Getting started guide
- [AGENTS.md](../AGENTS.md) - Detailed architecture and development guide
- [Contributing Guide](../docs/contrib.md) - Contribution guidelines
- [Configuration Options](../docs/configuration_options.md) - All configuration parameters
- [Commitlint Config](../commitlint.config.js) - Commit message validation rules
- [Pre-commit Config](../.pre-commit-config.yaml) - Pre-commit hook configuration
- [Pytest Config](../unit_tests/pytest.ini) - Pytest configuration and markers
