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

fix: Allow creating shelf files again #1576

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

AiyionPrime
Copy link
Contributor

@AiyionPrime AiyionPrime commented Feb 13, 2024

No description provided.

@oroulet
Copy link
Member

oroulet commented Feb 16, 2024

I am fine with these things, I have never used that shelv thing, so if it work for you I am fine with merging it

@AiyionPrime AiyionPrime force-pushed the fix/creating_shelf_files branch 2 times, most recently from 8c2a3d4 to 9283b37 Compare February 16, 2024 12:48
@AiyionPrime AiyionPrime marked this pull request as ready for review February 16, 2024 12:48
@@ -187,9 +187,6 @@ def test_sync_server_get_node(server, idx):
assert vars[0].read_value() == 6.7


@pytest.mark.xfail(
raises=AttributeError, reason="asyncua introduced a regression, likely when we switched to pathlib", strict=True
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now this strict=True would have led your pipeline to fail, if I resolved the issue, without removing this xfail marker again.

All in all this is a rather clean structure for test driven development with pytest, which I've learned to value over time.

in internal_server and address_space.
Path.is_file does not consume strings,
as shelf.open does only consume string and not pathlib.Path.
@AiyionPrime
Copy link
Contributor Author

@oroulet all tests passed, this also is ready to merge.

@oroulet oroulet merged commit 26bf738 into FreeOpcUa:master Feb 16, 2024
5 checks passed
@AiyionPrime AiyionPrime deleted the fix/creating_shelf_files branch February 16, 2024 14:24
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