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

Add implementation for overridden tests #69

Open
jescalada opened this issue Oct 19, 2024 · 1 comment
Open

Add implementation for overridden tests #69

jescalada opened this issue Oct 19, 2024 · 1 comment

Comments

@jescalada
Copy link
Contributor

Currently, some of the tests in the original test_sqlalchemy_store.py are failing due to various reasons. We decided to temporarily override those with empty tests, however the best solution is to reimplement the tests and remove the portions that fail.

The main issue, is that these tests are dependent on other private helper functions which we should also reimplement (such as _run_factory and _create_experiments).

For example, this is the original test_set_tag, which fails due to the monkeypatch env setup not working:

def test_set_tag(store: SqlAlchemyStore, monkeypatch):
    run = _run_factory(store)

    tkey = "test tag"
    tval = "a boogie"
    new_val = "new val"
    tag = entities.RunTag(tkey, tval)
    new_tag = entities.RunTag(tkey, new_val)
    store.set_tag(run.info.run_id, tag)
    # Overwriting tags is allowed
    store.set_tag(run.info.run_id, new_tag)
    # test setting tags that are too long fails.
    monkeypatch.setenv("MLFLOW_TRUNCATE_LONG_VALUES", "false")
    with pytest.raises(
        MlflowException, match=f"exceeds the maximum length of {MAX_TAG_VAL_LENGTH} characters"
    ):
        store.set_tag(
            run.info.run_id, entities.RunTag("longTagKey", "a" * (MAX_TAG_VAL_LENGTH + 1))
        )

    monkeypatch.setenv("MLFLOW_TRUNCATE_LONG_VALUES", "true")
    store.set_tag(run.info.run_id, entities.RunTag("longTagKey", "a" * (MAX_TAG_VAL_LENGTH + 1))) # <--- Fails here

    # test can set tags that are somewhat long
    store.set_tag(run.info.run_id, entities.RunTag("longTagKey", "a" * (MAX_TAG_VAL_LENGTH - 1)))
    run = store.get_run(run.info.run_id)
    assert tkey in run.data.tags
    assert run.data.tags[tkey] == new_val

Once the helpers are implemented, we can simply remove the failing line and the calls to monkeypatch which don't work. Then, we can repeat the process for the rest of the overridden tests.

@nojaf
Copy link
Collaborator

nojaf commented Oct 19, 2024

such as _run_factory and _create_experiments

Not sure those are problematic, the monkeypatch.setenv is most likely the real culprit.

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

No branches or pull requests

2 participants