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.

Sign up for GitHub

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

Jump to bottom

test: fully cover sopel.tools.target #2452

Merged
merged 2 commits into from
May 30, 2023
Merged

test: fully cover sopel.tools.target #2452

merged 2 commits into from
May 30, 2023

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented May 6, 2023

Description

Tin. I know this might have some conflict with #2448 but that's OK, I'll fix them on time.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

@Exirel Exirel added Low Priority Tests Housekeeping Code cleanup, removal of deprecated stuff, etc. labels May 6, 2023
@Exirel Exirel added this to the 8.0.0 milestone May 6, 2023
@Exirel Exirel force-pushed the test-tools-target branch from 4af3e6b to 0670cf5 Compare May 6, 2023 19:53
SnoopJ
SnoopJ reviewed May 24, 2023
View reviewed changes
test/tools/test_tools_target.py Outdated Show resolved Hide resolved
test/tools/test_tools_target.py Outdated Show resolved Hide resolved
test/tools/test_tools_target.py Show resolved Hide resolved
sopel/tools/target.py Outdated Show resolved Hide resolved
dgw
dgw approved these changes May 29, 2023
View reviewed changes
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

If @SnoopJ's happy, I'm happy. (Because I was already happy before Snoo— oh, you know.)

Just dropped a couple hints to help with the rebase in line notes. 🙂

sopel/tools/target.py Outdated Show resolved Hide resolved
test/tools/test_tools_target.py Show resolved Hide resolved
@Exirel
Copy link
Contributor Author

Exirel commented May 29, 2023

Rebase incoming...

@Exirel Exirel force-pushed the test-tools-target branch 2 times, most recently from ae94eed to d58d949 Compare May 29, 2023 22:05
@Exirel
Copy link
Contributor Author

Exirel commented May 29, 2023

So I did that with 2 force-push:

  • a squash so you can see it first if you want to
  • then an actual rebase upon master to resolve the conflicts, which were minimal

github-advanced-security[bot]
github-advanced-security bot found potential problems May 29, 2023
View reviewed changes
test/tools/test_tools_target.py

arthur = Identifier('Arthur')
with pytest.raises(TypeError):
arthur < brian

Check notice

Code scanning / CodeQL

Statement has no effect

This statement has no effect.
test/tools/test_tools_target.py

blasphemy = Identifier('#blasphemy')
with pytest.raises(TypeError):
blasphemy < channel

Check notice

Code scanning / CodeQL

Statement has no effect

This statement has no effect.
Exirel and others added 2 commits May 30, 2023 10:20
@Exirel Exirel force-pushed the test-tools-target branch from eb13866 to 855f827 Compare May 30, 2023 08:20
@Exirel
Copy link
Contributor Author

Exirel commented May 30, 2023

Rebase, squashed, all done for @dgw to merge.

@dgw dgw merged commit 7179686 into sopel-irc:master May 30, 2023
@Exirel Exirel deleted the test-tools-target branch June 15, 2023 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgw dgw dgw approved these changes

@SnoopJ SnoopJ SnoopJ approved these changes

Assignees
No one assigned
Labels
Housekeeping Code cleanup, removal of deprecated stuff, etc. Low Priority Tests
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Exirel @dgw @SnoopJ