spectacles

spectacles copied to clipboard

We are trying to run all the tests against any modified explorers in a branch, but running \n spectacles assert --branch BRANCH_NAME --explores a/b fails if there are no data tests to run.

I would expect the assert test to pass successfully if there are no data tests while giving a warning indicating that nothing was done.

Is there any reason why the assert test should fail if there are no data tests?

Hi @leoch20. We've had some internal debates about the right behavior here (see #227). Our current thinking is that we don't want people to accidentally merge LookML that should've failed tests but didn't because they specified the wrong models or explores (otherwise why are you running spectacles assert?)

However, it's clear that in your use case (and in another recently raised), you're providing --explores programmatically so you don't always know up front if there will be tests or not. That's a pretty good argument in favor of not raising an error.

I'm curious to hear your thoughts. Also, out of curiosity, how are you determining which explores are modified?

Hi @joshtemple I'm using the GitHub API to get all the files that belong to a PR. Then I have a slightly over engineered process to determine what model/explores belong to each file. E.g. if I have a dates view and I make some update to the file I want to test all the explores that use the view. This exercise narrows down the number of tests I'd need to run.

Using the same example, the dates view will return many explores, some of witch might have already a data test, some might have it in the future, and some might never do. The problem is that I do not know which explores have a test.

Maybe both use cases could be addressed with a --ignore-missing-test flag (or another shorter name) that would not make the process fail if there are no data tests for a given explore. Any thoughts?

Hello There ! Encountering the same issue, wondering if you had given more thoughts to that internally ? We encounter the same case :)

We’ve just been revisiting this in some internal discussions. The plan is to make this a warning and not an error, but it may take some time for us to fully implement it.

On Wed, May 24, 2023 at 08:36 Amaury Faure @.***> wrote:

Hello There ! Encountering the same issue, wondering if you had given more thoughts to that internally ? We encounter the same case :)

— Reply to this email directly, view it on GitHub https://github.com/spectacles-ci/spectacles/issues/451#issuecomment-1561049998, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCFHK4YUCI2DFB7DIDTGGTXHX6GPANCNFSM5HYETJ5A . You are receiving this because you were mentioned.Message ID: @.***>

Okay ! Following this then :) Thanks for the answer

Hello ! Following up on this, did you have the time to implement that ?