Skip to content
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

Refactor tests for Process #33

Merged
merged 4 commits into from
Apr 8, 2024
Merged

Conversation

jptomoya
Copy link
Contributor

@jptomoya jptomoya commented Apr 8, 2024

Refactor tests for Process

What's this Pull Request for?

Choose one or more of the following items.

  • Bug fix
  • Add/Remove feature
  • Cleaning code (including optimization/typo fix)
  • Others

Please explain the detail here:

  • Refactor tests for Process and improved exception handling using assertRaises context manager
    • Added checks to ensure that the process under test is successfully created before the test starts
    • Fixed tests in test_fsb that were not functioning correctly due to assertions always succeeding
  • Fixed a ResourceWarning in tests.pwn.test_fsb.TestFSB
  • Assigned the actual implementation for Process directly instead of using a wrapper function, slightly reducing overhead

What have you done so far?

Tell me what you've tested to assure your change is right.

  • All tests passed on your machine (python -m unittest)
  • Added test cases for new feature
  • Nothing / Test not required

(It's not mandatory to check even one of them, but test cases make it easy for the author to review your PR.)

Comment

Hava a nice day :)

@ptr-yudai
Copy link
Owner

Thanks again for your PR.

Fixed tests in test_fsb that were not functioning correctly

good catch :)

@ptr-yudai ptr-yudai merged commit 6579a72 into ptr-yudai:master Apr 8, 2024
10 checks passed
@jptomoya jptomoya deleted the refactor-test branch April 9, 2024 05:06
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.

2 participants