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

Make a new test for proper edge merging #411

Closed
tokebe opened this issue Feb 17, 2022 · 4 comments
Closed

Make a new test for proper edge merging #411

tokebe opened this issue Feb 17, 2022 · 4 comments
Labels
good first issue Good for newcomers

Comments

@tokebe
Copy link
Member

tokebe commented Feb 17, 2022

As noted by @andrewsu, #390 is a good issue to turn into a module-level test for proper edge merging.

This query is a good test query to create simplified internal objects for this test
{
    "message": {
        "query_graph": {
            "edges": {
                "e00": {
                    "subject": "n00",
                    "object": "n01",
                    "predicates": ["biolink:affects_risk_for"]
                }
            },
            "nodes": {
                "n00": {
                    "categories": ["biolink:SmallMolecule"],
                    "ids": ["UMLS:C0040329"]
                },
                "n01": {
                    "categories": ["biolink:Disease"],
                    "ids": ["UMLS:C0242379"]
                }
            }
        }
    }
}
@tokebe tokebe added the good first issue Good for newcomers label Jul 18, 2022
@rjawesome
Copy link
Contributor

@tokebe What exactly is a simplified internal object and how would I go about creating/testing it?

@tokebe
Copy link
Member Author

tokebe commented Jul 19, 2022

@rjawesome You can save Record objects, which are created from the responses of queried APIs and used throughout results-assembly, etc., by running BTE with the environment variable DUMP_RECORDS=path/to/fileToCreate.json (or by pausing execution after Records have been created+annotated and using built-in Record methods, though this is a little more complicated).

You'll want to write this test in the query_graph_handler test files -- The behavior you'll be testing was fixed by this PR, which hopefully gives an idea of where to start. If not, shoot me a message and I can try to clarify what part of query execution this test should be focused on.

From here you can write a test which supplies these records and expects specific edge-merging behavior (as shown in the original issue). You'll need to import the Record class and "un-freeze" your saved objects.

@rjawesome
Copy link
Contributor

rjawesome commented Jul 19, 2022

@tokebe To confirm, these are the two things to test:

  1. Test the behavior of the is_set parameter
  2. Test that same records with different predicates aren't merged?

Is there anything else that I am missing?

@tokebe
Copy link
Member Author

tokebe commented Jul 19, 2022

You only need to test that records that are otherwise the same but have different predicates are not merged into one response edge/result. No worries for is_set as far as this issue is concerned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants