Bitmessage/PyBitmessage-2024-08-21
Archived
2
0
Fork 0
Code Issues 168 Pull Requests 53 Actions Packages Projects Releases 5 Wiki Activity

A working subset of #1788 #1875

Merged
g1itch merged 5 commits from compat-short into v0.6 2021-11-11 15:12:34 +01:00
Conversation 2 Commits 5 Files Changed 5 +112 -10
g1itch commented 2021-11-02 17:34:12 +01:00 (Migrated from github.com)
Copy Link

Hello!

This is a subset of #1788, the tests that are ready (+4 tests, including those for base58 and varint) and corresponding changes in the protocol module.

Hello! This is a subset of #1788, the tests that are ready (+4 tests, including those for base58 and varint) and corresponding changes in the `protocol` module.
g1itch commented 2021-11-02 17:42:22 +01:00 (Migrated from github.com)
Copy Link

There is also a test for base58 in #1796: 7f213f6

There is also a test for base58 in #1796: 7f213f6
PeterSurda (Migrated from github.com) reviewed 2021-11-03 08:15:58 +01:00
src/tests/test_addresses.py
PeterSurda (Migrated from github.com) commented 2021-11-03 08:15:58 +01:00
  • these strings and numbers should be in samples.py
  • to improve coverage, encode needs to test 0, and decode needs to test an invalid character
- these strings and numbers should be in `samples.py` - to improve coverage, encode needs to test `0`, and decode needs to test an invalid character
PeterSurda (Migrated from github.com) reviewed 2021-11-03 08:17:09 +01:00
src/tests/test_packets.py
PeterSurda (Migrated from github.com) commented 2021-11-03 08:17:09 +01:00

length, not lenght

`length`, not `lenght`
PeterSurda (Migrated from github.com) requested changes 2021-11-03 08:19:20 +01:00
PeterSurda (Migrated from github.com) left a comment

Mostly ok, just minor cleanup pls.

Mostly ok, just minor cleanup pls.
g1itch (Migrated from github.com) reviewed 2021-11-03 16:18:15 +01:00
src/tests/test_addresses.py
g1itch (Migrated from github.com) commented 2021-11-03 16:18:15 +01:00

@PeterSurda what do you think addresses.encodeBase58() should do in such case? Raise an exception, return None, or drop the sign?

@PeterSurda what do you think `addresses.encodeBase58()` should do in such case? Raise an exception, return None, or drop the sign?
g1itch (Migrated from github.com) reviewed 2021-11-03 16:18:53 +01:00
src/tests/test_addresses.py
g1itch (Migrated from github.com) commented 2021-11-03 16:18:53 +01:00

This is the invalid character.

This is the invalid character.
PeterSurda (Migrated from github.com) reviewed 2021-11-04 03:47:24 +01:00
src/tests/test_addresses.py
PeterSurda (Migrated from github.com) commented 2021-11-04 03:47:24 +01:00

Good catch, I'd go with return None.

Good catch, I'd go with `return None`.
PeterSurda (Migrated from github.com) requested changes 2021-11-04 07:07:07 +01:00
PeterSurda (Migrated from github.com) left a comment

Still encodeBase58(0) is missing. Fixing negative can be fixed either in this PR or a separate one.

Still `encodeBase58(0)` is missing. Fixing negative can be fixed either in this PR or a separate one.
src/tests/test_addresses.py
PeterSurda (Migrated from github.com) commented 2021-11-04 07:06:22 +01:00

still missing test for 0:
self.assertEqual(addresses.encodeBase58(0), '1')

still missing test for `0`: `self.assertEqual(addresses.encodeBase58(0), '1')`
g1itch (Migrated from github.com) reviewed 2021-11-04 14:44:29 +01:00
src/tests/test_addresses.py
g1itch (Migrated from github.com) commented 2021-11-04 14:44:29 +01:00

It failed when I wrote it first time, maybe I misspelled something

It failed when I wrote it first time, maybe I misspelled something
g1itch commented 2021-11-04 17:22:00 +01:00 (Migrated from github.com)
Copy Link

Fixing negative can be fixed either in this PR or a separate one.

It's a simple mechanical fix. I prefer to merge complete tests, not stubs.

> Fixing negative can be fixed either in this PR or a separate one. It's a simple mechanical fix. I prefer to merge complete tests, not stubs.
PeterSurda (Migrated from github.com) approved these changes 2021-11-11 08:13:08 +01:00
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
PeterSurda
Labels
Clear labels   
bug

Something isn't working as it's supposed to

  
build

Related to building or build system

  
dependencies

Pull requests that update a dependency file

  
developers

The issue is relevant mainly for developers rather than users

  
documentation

   
duplicate

   
enhancement

New feature

  
formatting

Code or UI text formatting

  
invalid

   
legal

   
mobile

Frontend for mobile devices (kivy)

  
obsolete

   
packaging

   
performance

   
protocol

Protocol change

  
question

   
refactoring

   
regression

Some issue, fixed in the past, appears again

  
security

   
test

   
translation

   
usability

   
wontfix
No Label
bug
build
dependencies
developers
documentation
duplicate
enhancement
formatting
invalid
legal
mobile
obsolete
packaging
performance
protocol
question
refactoring
regression
security
test
translation
usability
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Bitmessage/PyBitmessage-2024-08-21#1875
No description provided.
Delete Branch "compat-short"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?