Skip to content

fix: remove misleading second arg from assertTrue/assertFalse calls#1381

Merged
bact merged 3 commits intoPyThaiNLP:devfrom
phoneee:fix/test-morpheme-assertions
Mar 31, 2026
Merged

fix: remove misleading second arg from assertTrue/assertFalse calls#1381
bact merged 3 commits intoPyThaiNLP:devfrom
phoneee:fix/test-morpheme-assertions

Conversation

@phoneee
Copy link
Copy Markdown
Contributor

@phoneee phoneee commented Mar 29, 2026

What do these changes do

Fix assertTrue(result, True) in test_morpheme — the second arg is the failure message, not the expected value

  • Passed code styles and structures
  • Passed code linting checks and unit test

assertTrue(expr, True) passes True as the failure message parameter,
not as the expected value. The tests passed accidentally because
is_native_thai returns a truthy value. Remove the second argument
so assertions properly test the return value.
@bact bact added this to PyThaiNLP Mar 29, 2026
@bact bact added the bug bugs in the library label Mar 29, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes misleading boolean “second arguments” from several unittest.TestCase.assertTrue/assertFalse calls in test_morpheme, where the second parameter is actually a failure message (msg), not an expected value.

Changes:

  • Updated test_is_native_thai assertions to call assertTrue(...) / assertFalse(...) with only the condition.
  • Reduced confusion in tests by avoiding incorrect “expected value” style usage.

@bact bact added the tests Unit test, code coverage, test case label Mar 30, 2026
@bact bact moved this to In progress in PyThaiNLP Mar 30, 2026
@sonarqubecloud
Copy link
Copy Markdown

@bact bact merged commit acc9360 into PyThaiNLP:dev Mar 31, 2026
24 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in PyThaiNLP Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug bugs in the library tests Unit test, code coverage, test case

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants