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

Specific Exceptions: Adapting pcan interface #1152

Merged
merged 3 commits into from
Dec 7, 2021

Conversation

felixdivo
Copy link
Collaborator

Part of #1046.

Also partly cleans up the docstrings in basic.py and uses some subtests in test_pcan.py (the parameter name was else unused in most parameterized tests).

@felixdivo felixdivo added this to the 4.0.0 Release milestone Oct 27, 2021
@felixdivo felixdivo self-assigned this Oct 27, 2021
@mergify mergify bot requested a review from hardbyte October 27, 2021 14:08
@codecov
Copy link

codecov bot commented Oct 27, 2021

Codecov Report

Merging #1152 (02b288d) into develop (9080c47) will increase coverage by 0.01%.
The diff coverage is 82.60%.

@@             Coverage Diff             @@
##           develop    #1152      +/-   ##
===========================================
+ Coverage    70.86%   70.88%   +0.01%     
===========================================
  Files           79       79              
  Lines         7765     7770       +5     
===========================================
+ Hits          5503     5508       +5     
  Misses        2262     2262              

can/interfaces/pcan/pcan.py Outdated Show resolved Hide resolved
"""
if self.status() in PCAN_DICT_STATUS:
try:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change prevents a hypothetical error if self.status() changes between lines.

@felixdivo
Copy link
Collaborator Author

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Nov 15, 2021

rebase

✅ Branch has been successfully rebased

@felixdivo
Copy link
Collaborator Author

What a weird failure:

_____________ TestBaseRotatingLogger.test_rotate_without_rotator ______________

self = <test.test_rotating_loggers.TestBaseRotatingLogger object at 0x000001b81f183590>
tmp_path = WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-unknown/pytest-0/test_rotate_without_rotator0')

    def test_rotate_without_rotator(self, tmp_path):
        logger_instance = self._get_instance(tmp_path)
    
        source = str(tmp_path / "source.txt")
        dest = str(tmp_path / "dest.txt")
    
        assert os.path.exists(source) is False
        assert os.path.exists(dest) is False
    
        logger_instance._get_new_writer(source)
        logger_instance.stop()
    
        assert os.path.exists(source) is True
        assert os.path.exists(dest) is False
    
>       logger_instance.rotate(source, dest)

test\test_rotating_loggers.py:99: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <test.test_rotating_loggers.TestBaseRotatingLogger._get_instance.<locals>.SubClass object at 0x000001b81f183a28>
source = 'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\pytest-of-unknown\\pytest-0\\test_rotate_without_rotator0\\source.txt'
dest = 'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\pytest-of-unknown\\pytest-0\\test_rotate_without_rotator0\\dest.txt'

    def rotate(self, source: StringPathLike, dest: StringPathLike) -> None:
        """When rotating, rotate the current log.
    
        The default implementation calls the :attr:`rotator` attribute of the
        handler, if it's callable, passing the source and dest arguments to
        it. If the attribute isn't callable (the default is `None`), the source
        is simply renamed to the destination.
    
        :param source:
            The source filename. This is normally the base
            filename, e.g. `"test.log"`
        :param dest:
            The destination filename. This is normally
            what the source is rotated to, e.g. `"test_#001.log"`.
        """
        if not callable(self.rotator):
            if os.path.exists(source):
>               os.rename(source, dest)
E               PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\pytest-of-unknown\\pytest-0\\test_rotate_without_rotator0\\source.txt' -> 'C:\\Users\\runneradmin\\AppData\\Local\\Temp\\pytest-of-unknown\\pytest-0\\test_rotate_without_rotator0\\dest.txt'

can\io\logger.py:177: PermissionError

@felixdivo
Copy link
Collaborator Author

What a weird failure:

Will be fixed by a6256e8 as soon as #1166 is merged

@felixdivo
Copy link
Collaborator Author

@Mergifyio rebase

Part of #1046.

Also partly cleans up the docstrings in `basic.py` and uses some subtests in `test_pcan.py` (the parameter `name` was else unused in most parameterized tests).
@mergify
Copy link
Contributor

mergify bot commented Nov 16, 2021

rebase

✅ Branch has been successfully rebased

@felixdivo
Copy link
Collaborator Author

@hardbyte Do you think you'll have time to review the open RPs for the 4.0.0 Release in the near future? There are also a few PRs that are approved by others and are waiting to be merged. It would be nice to include them into the next release (but the main blocker are the PRs around Specific Exceptions).

@felixdivo felixdivo merged commit d603ff9 into develop Dec 7, 2021
@felixdivo felixdivo deleted the felixdivo-issue-1046-pcan branch December 7, 2021 19:07
cowo78 pushed a commit to cowo78/python-can that referenced this pull request Dec 9, 2021
* Specific Exceptions: Adapting pcan interface

Part of hardbyte#1046.

Also partly cleans up the docstrings in `basic.py` and uses some subtests in `test_pcan.py` (the parameter `name` was else unused in most parameterized tests).

* Run black formatter

* Remove wildcard import; move some dicts/lists to basic.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants