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

TST: add missing dependency that reveals failing tests from pcds-envs #166

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

ZLLentz
Copy link
Member

@ZLLentz ZLLentz commented Aug 21, 2023

Description

Add blark as a dev/test dependency, in an attempt to reveal a test that fails on pcds-envs but doesn't run at all in the test suite here.

Motivation and Context

In pcds-envs on the weekly build, whatrecord was failing with this message:

_________________________ test_serialize[PlcMetadata] __________________________

cls = <class 'whatrecord.plugins.twincat_pytmc.PlcMetadata'>

    @all_dataclasses
    def test_serialize(cls):
>       instance = try_to_instantiate(cls)

whatrecord/tests/test_serialization.py:151: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
whatrecord/tests/test_serialization.py:123: in try_to_instantiate
    type_hints = apischema.utils.get_type_hints(cls)
/home/runner/miniconda/envs/pcds-test/lib/python3.9/typing.py:1459: in get_type_hints
    value = _eval_type(value, base_globals, localns)
/home/runner/miniconda/envs/pcds-test/lib/python3.9/typing.py:292: in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
/home/runner/miniconda/envs/pcds-test/lib/python3.9/typing.py:554: in _evaluate
    eval(self.__forward_code__, globalns, localns),
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

>   ???
E   AttributeError: module 'blark.dependency_store' has no attribute 'ResolvedDependency'

I checked this repo and found that this test never gets run, and I think it's because there's an import error raised here:

logger.exception("Failed to import %s", item.name)
in importing whatrecord.plugins.twincat_pytmc here:
from blark import dependency_store

Which is because blark isn't installed in the during the test suite.

How Has This Been Tested?

n/a, I just want to see if the test that fails on pcds-envs also fails on the base whatrecord repo
it's good to catch these earlier and I think all tests being run is the desired behavior

Where Has This Been Documented?

n/a

@ZLLentz
Copy link
Member Author

ZLLentz commented Aug 21, 2023

OK, yes, this test does fail on whatrecord too if it gets run, not only in the context of pcds-envs, which is a relief because that will make it simpler to track down.

I'm not sure immediately what to do with this PR- I've accomplished my short-term goal of restoring a missing test from the suite.

There are more missing tests to track down. Here's the final line from pcds-envs:

= 1 failed, 428 passed, 2 skipped, 7 xfailed, 10 warnings in 342.16s (0:05:42) =

Here's the final line from whatrecord in this PR:

= 1 failed, 409 passed, 20 skipped, 6 xfailed, 34 warnings in 438.95s (0:07:18) =

Here's the final line from whatrecord before this PR:

===== 403 passed, 20 skipped, 6 xfailed, 34 warnings in 445.79s (0:07:25) ======

@klauer
Copy link
Contributor

klauer commented Aug 21, 2023

There's a bit of work to be done to get whatrecord working with blark again after a big refactor there.

I'm afraid the whatrecord test suite will either have to be added to xfail or blark would have to be pinned to a compatible version (but that'd be less than preferable since the later versions of blark have a lot better source compatibility)

@ZLLentz
Copy link
Member Author

ZLLentz commented Aug 21, 2023

Ok, I'll just leave this sitting here then until we decide what to do. I think it's a major structural problem that specific tests don't get run at all, it gives off a false impression that everything is working fully. It's much better to explicitly skip/xfail them instead of not run them.

@klauer
Copy link
Contributor

klauer commented Aug 21, 2023

I think it's a major structural problem that specific tests don't get run at all, it gives off a false impression that everything is working fully.

Agreed - but such is the nature of these side project monstrosities...

@klauer klauer merged commit 6c693db into pcdshub:master Aug 23, 2023
3 of 9 checks passed
@ZLLentz ZLLentz deleted the tst_missing_dep branch August 23, 2023 20:07
@klauer
Copy link
Contributor

klauer commented Aug 23, 2023

This was merged, but the fix won't be applied until the next tag.
Blocker for next tag: #162

This pull request was closed.
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