Skip to content

Add unit tests for core functions #10

Description

@netanelcyber

Description

Create comprehensive unit test suite for core framework functions.

Scope

  • smtp_vrfy_enum() - SMTP VRFY user enumeration
  • smtp_rcpt_enum() - SMTP RCPT TO validation
  • smtp_auth_test() - SMTP credential testing
  • pop3_auth_test() - POP3 authentication
  • imap_auth_test() - IMAP authentication
  • detect_dcs() - Multi-strategy DC discovery
  • build_ad_command() - Tool-specific command generation
  • execute_ad_tool() - Subprocess execution
  • discover_tools() - Tool availability detection

Requirements

  • Use pytest framework
  • Mock external services (SMTP, LDAP, DNS)
  • Achieve 80%+ code coverage on core functions
  • Test error scenarios and edge cases
  • Test timeout handling

Files to Update

  • Create: tests/test_core.py
  • Create: tests/conftest.py (pytest fixtures)
  • Create: tests/mocks/ (mock services)

Success Criteria

  • All tests pass locally
  • CI/CD runs tests automatically
  • Coverage report generated

Review update — 2026-09-11

PR #47 was merged as b4da033116c48e80c8480bdfd792adc638dc4d4b on September 11 at 06:02 UTC. This review is pinned to that main snapshot (version 1.2.4), not to a claim that the PR is still unmergeable. The PR's three test files landed unchanged. A local merge-tree check against pre-merge main d6abf50 completed without conflicts.

Reproduced result

Running tests/test_core.py with external process launches blocked by a patched subprocess.Popen raising OSError, and socket connections blocked, produced 16 passed, 1 failed. The timeout test received execution-error; the purported success test passed with that same execution error because it only checks for a status key. This was a guarded local run, not an unmodified CI run. Coverage was not measured.

Minimum test/CI follow-up

  • Mock smtp.rcpt(), not rcpt_to() or verify(), in the RCPT tests. Assert exact returned email addresses and recipient calls.
  • Inject SMTP connection failures at the smtplib.SMTP constructor, rather than at login/verification.
  • Mock adpentest.core.subprocess.Popen, which execution actually uses. For success, configure poll() and communicate(); assert completed, stdout, stderr and exit code.
  • Simulate a running process and elapsed time for timeout; assert kill() and exact status timeout. The existing builtin TimeoutError on the unused subprocess.run mock does not exercise timeout handling.
  • Exercise smtp_connect_test() with the socket fixture and assert actual return values; strengthen discovery assertions to exact available/unavailable sets.
  • Install pytest and run python -m pytest -q tests/test_core.py in the existing Windows/Ubuntu CI matrix. The reviewed workflow only compiles and invokes a dry-run.
  • Correct the coverage description: PR Add unit tests for core functions (fixes #10) #47 does not contain the claimed VRFY negative/connection-error tests or POP3/IMAP tests.

Remaining original acceptance criteria

The minimum corrections above do not complete this issue's broader scope. POP3/IMAP and DC-discovery coverage, a generated coverage report, and evidence for the original 80%+ core-function target remain outstanding. Keep this issue open until those criteria are verified. Coordinate broader CI/coverage work with #31.

Sources: PR #47, reviewed tests, reviewed implementation, reviewed CI.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions