565 lines
12 KiB
Markdown
565 lines
12 KiB
Markdown
# Contributing to @lumbridgecorp/openclaw-mattermost
|
|
|
|
Thank you for your interest in contributing! This document provides guidelines for setting up your development environment, running tests, and submitting contributions.
|
|
|
|
## Table of Contents
|
|
|
|
- [Development Setup](#development-setup)
|
|
- [Running Tests](#running-tests)
|
|
- [Test Structure](#test-structure)
|
|
- [Writing Tests](#writing-tests)
|
|
- [Code Style](#code-style)
|
|
- [Pull Request Process](#pull-request-process)
|
|
- [Release Process](#release-process)
|
|
|
|
## Development Setup
|
|
|
|
### Prerequisites
|
|
|
|
- **Node.js 22+** (required for native fetch and WebSocket support)
|
|
- **pnpm** (preferred) or npm/yarn
|
|
- **Git**
|
|
- **Mattermost server** (for integration tests)
|
|
|
|
### Local Setup
|
|
|
|
1. **Fork the repository** on GitHub
|
|
|
|
2. **Clone your fork:**
|
|
```bash
|
|
git clone https://github.com/YOUR_USERNAME/openclaw-extentions-mattermost.git
|
|
cd openclaw-extentions-mattermost
|
|
```
|
|
|
|
3. **Install dependencies:**
|
|
```bash
|
|
pnpm install
|
|
# or
|
|
npm install
|
|
```
|
|
|
|
4. **Build the project:**
|
|
```bash
|
|
pnpm build
|
|
# or
|
|
npm run build
|
|
```
|
|
|
|
5. **Verify the build:**
|
|
```bash
|
|
pnpm typecheck
|
|
```
|
|
|
|
## Running Tests
|
|
|
|
### Quick Test Commands
|
|
|
|
```bash
|
|
# Run all tests
|
|
pnpm test
|
|
|
|
# Run tests in watch mode (for development)
|
|
pnpm test -- --watch
|
|
|
|
# Run tests with coverage
|
|
pnpm test -- --coverage
|
|
|
|
# Run a specific test file
|
|
pnpm test src/mattermost/client.test.ts
|
|
|
|
# Run tests matching a pattern
|
|
pnpm test -- -t "should create client"
|
|
|
|
# Run tests with verbose output
|
|
pnpm test -- --verbose
|
|
```
|
|
|
|
### Test Categories
|
|
|
|
#### Unit Tests
|
|
|
|
Fast, isolated tests that don't require external services:
|
|
|
|
```bash
|
|
# Run only unit tests (no integration)
|
|
pnpm test -- --testPathIgnorePatterns="integration|e2e"
|
|
```
|
|
|
|
Unit tests cover:
|
|
- Error handling classes
|
|
- Configuration validation
|
|
- Utility functions
|
|
- Retry logic
|
|
- Security checks
|
|
|
|
#### Integration Tests with Docker Compose
|
|
|
|
Integration tests use Docker Compose to spin up a complete Mattermost instance with PostgreSQL. This provides isolated, reproducible tests against a real Mattermost API.
|
|
|
|
**Setup and Run Integration Tests:**
|
|
|
|
```bash
|
|
# Start the test infrastructure
|
|
docker-compose -f tests/integration/docker-compose.test.yml up -d
|
|
|
|
# Wait for Mattermost to be ready (first startup may take 1-2 minutes)
|
|
docker-compose -f tests/integration/docker-compose.test.yml ps
|
|
|
|
# Run integration tests
|
|
pnpm test tests/integration/
|
|
|
|
# Run specific integration test file
|
|
pnpm test tests/integration/authentication.test.ts
|
|
|
|
# Run integration tests with verbose output
|
|
pnpm test tests/integration/ --verbose
|
|
|
|
# Clean up after tests
|
|
docker-compose -f tests/integration/docker-compose.test.yml down -v
|
|
```
|
|
|
|
**Integration Test Coverage:**
|
|
|
|
| Test File | Coverage |
|
|
|-----------|----------|
|
|
| `authentication.test.ts` | Bot token auth, PAT auth, error scenarios |
|
|
| `send-message.test.ts` | Send to channel/DM, threads, props, error cases |
|
|
| `edit-message.test.ts` | Edit own messages, props, permission errors |
|
|
| `delete-message.test.ts` | Delete messages, thread handling, cleanup |
|
|
| `download-file.test.ts` | File download, security restrictions, MIME validation |
|
|
| `interactive-buttons.test.ts` | Button rendering, directives, end-to-end flows |
|
|
|
|
**Environment Variables for Integration Tests:**
|
|
|
|
```bash
|
|
# Optional: Override defaults (only if not using Docker Compose)
|
|
export MATTERMOST_URL="http://localhost:8065"
|
|
export MATTERMOST_SYSADMIN_USER="sysadmin"
|
|
export MATTERMOST_SYSADMIN_PASSWORD="Sys@dmin123"
|
|
export MATTERMOST_TEST_USER="testuser"
|
|
export MATTERMOST_TEST_USER_PASSWORD="Test@user123"
|
|
export MATTERMOST_BOT_USERNAME="testbot"
|
|
```
|
|
|
|
**Test Data Cleanup:**
|
|
|
|
Integration tests automatically clean up:
|
|
- Created posts after each test suite
|
|
- Uploaded files
|
|
- Test channels and teams (via Docker volume reset)
|
|
|
|
Run `docker-compose -f tests/integration/docker-compose.test.yml down -v` to completely reset the test environment between runs.
|
|
|
|
**Using a Custom Mattermost Instance:**
|
|
|
|
If you prefer to use an existing Mattermost instance for integration tests:
|
|
|
|
```bash
|
|
# Set up environment variables
|
|
export MATTERMOST_URL="https://chat-test.example.com"
|
|
export MATTERMOST_SYSADMIN_TOKEN="your-sysadmin-token"
|
|
export MATTERMOST_BOT_TOKEN="your-bot-token"
|
|
|
|
# Run integration tests (skip Docker setup)
|
|
export SKIP_DOCKER_SETUP=true
|
|
pnpm test tests/integration/
|
|
```
|
|
|
|
⚠️ **Important:** Never run integration tests against production Mattermost instances. Tests create and delete data automatically.
|
|
|
|
#### Client Tests
|
|
|
|
Tests for the Mattermost API client:
|
|
|
|
```bash
|
|
pnpm test src/mattermost/client.test.ts
|
|
pnpm test src/mattermost/client.retry.test.ts
|
|
```
|
|
|
|
#### WebSocket Tests
|
|
|
|
Tests for real-time message monitoring:
|
|
|
|
```bash
|
|
pnpm test src/mattermost/monitor-websocket.test.ts
|
|
```
|
|
|
|
### Test Environment Setup
|
|
|
|
#### Using Environment Variables
|
|
|
|
Create a `.env.test` file (not committed to git):
|
|
|
|
```bash
|
|
# Test Mattermost instance
|
|
MATTERMOST_TEST_URL=https://chat-test.example.com
|
|
MATTERMOST_TEST_TOKEN=your-test-bot-token
|
|
MATTERMOST_TEST_PAT=your-test-pat-optional
|
|
|
|
# Test configuration
|
|
TEST_TIMEOUT=30000
|
|
TEST_RETRY_COUNT=3
|
|
```
|
|
|
|
#### Using Test Fixtures
|
|
|
|
Some tests use mock fixtures instead of real connections:
|
|
|
|
```typescript
|
|
// Example: Using mock client
|
|
import { createMockMattermostClient } from './test-helpers.js';
|
|
|
|
const mockClient = createMockMattermostClient({
|
|
baseUrl: 'https://mock.example.com',
|
|
botToken: 'mock-token'
|
|
});
|
|
```
|
|
|
|
### Coverage Reports
|
|
|
|
Generate and view coverage:
|
|
|
|
```bash
|
|
# Generate coverage report
|
|
pnpm test -- --coverage
|
|
|
|
# View HTML report
|
|
open coverage/lcov-report/index.html
|
|
|
|
# Coverage thresholds (enforced in CI)
|
|
# Statements: 80%
|
|
# Branches: 75%
|
|
# Functions: 80%
|
|
# Lines: 80%
|
|
```
|
|
|
|
## Test Structure
|
|
|
|
### Test File Organization
|
|
|
|
```
|
|
extensions/mattermost/src/
|
|
├── mattermost/
|
|
│ ├── client.test.ts # Client unit tests
|
|
│ ├── client.retry.test.ts # Retry logic tests
|
|
│ ├── monitor-websocket.test.ts # WebSocket tests
|
|
│ ├── interactions.test.ts # Interactive components tests
|
|
│ └── ...
|
|
├── config-schema.test.ts # Configuration validation
|
|
├── security-audit.test.ts # Security audit tests
|
|
├── errors.test.ts # Error handling tests
|
|
└── ...
|
|
```
|
|
|
|
### Test Naming Conventions
|
|
|
|
- **File naming**: `*.test.ts` for unit tests, `*.integration.test.ts` for integration tests
|
|
- **Describe blocks**: Use the name of the function/module being tested
|
|
- **Test names**: Start with "should" and describe the expected behavior
|
|
|
|
Example:
|
|
|
|
```typescript
|
|
describe('editMessage', () => {
|
|
it('should successfully edit a bot message', async () => {
|
|
// test code
|
|
});
|
|
|
|
it('should fail when editing another users message', async () => {
|
|
// test code
|
|
});
|
|
|
|
it('should return PERMISSION_DENIED for system messages', async () => {
|
|
// test code
|
|
});
|
|
});
|
|
```
|
|
|
|
## Writing Tests
|
|
|
|
### Unit Test Template
|
|
|
|
```typescript
|
|
import { describe, it, expect, vi } from 'vitest';
|
|
import { editMessage } from './actions.js';
|
|
|
|
describe('editMessage', () => {
|
|
it('should edit a message successfully', async () => {
|
|
// Arrange
|
|
const mockClient = createMockClient();
|
|
const params = {
|
|
cfg: mockConfig,
|
|
postId: 'post123',
|
|
channelId: 'channel123',
|
|
message: 'Updated'
|
|
};
|
|
|
|
// Act
|
|
const result = await editMessage(params);
|
|
|
|
// Assert
|
|
expect(result.ok).toBe(true);
|
|
expect(result.postId).toBe('post123');
|
|
});
|
|
|
|
it('should handle errors gracefully', async () => {
|
|
// Arrange
|
|
const params = { /* invalid params */ };
|
|
|
|
// Act
|
|
const result = await editMessage(params);
|
|
|
|
// Assert
|
|
expect(result.ok).toBe(false);
|
|
expect(result.errorCode).toBe('CONFIG_ERROR');
|
|
});
|
|
});
|
|
```
|
|
|
|
### Testing Error Handling
|
|
|
|
```typescript
|
|
import { MattermostError, ErrorCodes } from './errors.js';
|
|
|
|
describe('error handling', () => {
|
|
it('should create structured error with context', () => {
|
|
const error = new MattermostError({
|
|
code: ErrorCodes.AUTHENTICATION_FAILED,
|
|
message: 'Invalid token',
|
|
context: {
|
|
operation: 'test',
|
|
timestamp: new Date().toISOString()
|
|
}
|
|
});
|
|
|
|
expect(error.code).toBe(ErrorCodes.AUTHENTICATION_FAILED);
|
|
expect(error.retryable).toBe(false);
|
|
expect(error.toJSON()).toMatchObject({
|
|
code: ErrorCodes.AUTHENTICATION_FAILED
|
|
});
|
|
});
|
|
});
|
|
```
|
|
|
|
### Testing Retries
|
|
|
|
```typescript
|
|
import { withRetry } from './errors.js';
|
|
|
|
describe('withRetry', () => {
|
|
it('should retry on retryable errors', async () => {
|
|
let attempts = 0;
|
|
const operation = async () => {
|
|
attempts++;
|
|
if (attempts < 3) {
|
|
throw new NetworkError('Connection failed', context);
|
|
}
|
|
return 'success';
|
|
};
|
|
|
|
const result = await withRetry('test-op', operation, {
|
|
maxRetries: 3,
|
|
initialDelayMs: 10
|
|
});
|
|
|
|
expect(result).toBe('success');
|
|
expect(attempts).toBe(3);
|
|
});
|
|
});
|
|
```
|
|
|
|
### Testing Security Features
|
|
|
|
```typescript
|
|
import { collectMattermostSecurityAuditFindings } from './security-audit.js';
|
|
|
|
describe('security audit', () => {
|
|
it('should detect hardcoded tokens', async () => {
|
|
const findings = await collectMattermostSecurityAuditFindings({
|
|
cfg: mockConfig,
|
|
accountId: 'test',
|
|
account: {
|
|
...mockAccount,
|
|
botToken: 'hardcoded-token-123456789',
|
|
botTokenSource: 'config'
|
|
}
|
|
});
|
|
|
|
const hardcodedFinding = findings.find(
|
|
f => f.checkId === 'channels.mattermost.pat.hardcoded_token'
|
|
);
|
|
|
|
expect(hardcodedFinding).toBeDefined();
|
|
expect(hardcodedFinding?.severity).toBe('critical');
|
|
});
|
|
});
|
|
```
|
|
|
|
### Mocking the Mattermost Client
|
|
|
|
```typescript
|
|
import { vi } from 'vitest';
|
|
|
|
// Create mock client
|
|
const createMockClient = () => ({
|
|
request: vi.fn(),
|
|
fetchImpl: vi.fn(),
|
|
baseUrl: 'https://mock.example.com',
|
|
apiBaseUrl: 'https://mock.example.com/api/v4',
|
|
token: 'mock-token'
|
|
});
|
|
|
|
// Mock module
|
|
vi.mock('./client.js', () => ({
|
|
createMattermostClient: vi.fn(() => createMockClient())
|
|
}));
|
|
```
|
|
|
|
## Code Style
|
|
|
|
### TypeScript Guidelines
|
|
|
|
- Use strict TypeScript (`strict: true` in tsconfig)
|
|
- Explicit return types on public functions
|
|
- Avoid `any` - use `unknown` with type guards
|
|
- Use `readonly` for immutable properties
|
|
|
|
### Code Formatting
|
|
|
|
```bash
|
|
# Check code style
|
|
pnpm lint
|
|
|
|
# Fix auto-fixable issues
|
|
pnpm lint --fix
|
|
```
|
|
|
|
### Import Order
|
|
|
|
1. External dependencies (e.g., `openclaw/plugin-sdk`)
|
|
2. Internal modules (relative imports)
|
|
3. Types only imports
|
|
|
|
Example:
|
|
|
|
```typescript
|
|
// 1. External
|
|
import { z } from 'openclaw/plugin-sdk/zod';
|
|
|
|
// 2. Internal
|
|
import { createMattermostClient } from './client.js';
|
|
import type { MattermostAccountConfig } from './types.js';
|
|
```
|
|
|
|
## Pull Request Process
|
|
|
|
### Before Submitting
|
|
|
|
1. **Run all tests:**
|
|
```bash
|
|
pnpm test
|
|
```
|
|
|
|
2. **Check type safety:**
|
|
```bash
|
|
pnpm typecheck
|
|
```
|
|
|
|
3. **Run linter:**
|
|
```bash
|
|
pnpm lint
|
|
```
|
|
|
|
4. **Build project:**
|
|
```bash
|
|
pnpm build
|
|
```
|
|
|
|
5. **Check test coverage:**
|
|
```bash
|
|
pnpm test -- --coverage
|
|
```
|
|
|
|
### PR Requirements
|
|
|
|
- **Clear description** of changes
|
|
- **Link to related issues**
|
|
- **Tests included** for new features
|
|
- **Documentation updated** if needed
|
|
- **No breaking changes** (or clearly marked)
|
|
- **CI checks passing**
|
|
|
|
### PR Template
|
|
|
|
```markdown
|
|
## Description
|
|
Brief description of the change
|
|
|
|
## Type of Change
|
|
- [ ] Bug fix
|
|
- [ ] New feature
|
|
- [ ] Breaking change
|
|
- [ ] Documentation update
|
|
|
|
## Testing
|
|
- [ ] Unit tests added/updated
|
|
- [ ] Integration tests added/updated
|
|
- [ ] All tests passing
|
|
|
|
## Checklist
|
|
- [ ] Code follows style guidelines
|
|
- [ ] Self-review completed
|
|
- [ ] Documentation updated
|
|
- [ ] No new warnings
|
|
```
|
|
|
|
### Review Process
|
|
|
|
1. Automated checks must pass (CI, coverage)
|
|
2. At least one maintainer review required
|
|
3. Address review feedback promptly
|
|
4. Squash commits before merge (if requested)
|
|
|
|
## Release Process
|
|
|
|
### Version Bump
|
|
|
|
```bash
|
|
# Update version in package.json
|
|
npm version patch # or minor, major
|
|
```
|
|
|
|
### Changelog Update
|
|
|
|
Add entry to `CHANGELOG.md` following Keep a Changelog format:
|
|
|
|
```markdown
|
|
## [1.1.0] - 2026-04-15
|
|
|
|
### Added
|
|
- New feature description
|
|
|
|
### Fixed
|
|
- Bug fix description
|
|
```
|
|
|
|
### Release Steps
|
|
|
|
1. Update version in `package.json`
|
|
2. Update `CHANGELOG.md`
|
|
3. Create PR with version bump
|
|
4. After merge, tag the release:
|
|
```bash
|
|
git tag -a v1.1.0 -m "Release v1.1.0"
|
|
git push origin v1.1.0
|
|
```
|
|
5. CI will automatically publish to npm
|
|
|
|
## Questions?
|
|
|
|
- Open an issue on GitHub
|
|
- Check existing issues and discussions
|
|
- Review the [API Reference](docs/API.md)
|
|
|
|
Thank you for contributing!
|