alfert / propcheck

Property based Testing for Elixir (based upon PropEr)
GNU General Public License v3.0
376 stars 42 forks source link

Child app in umbrella looks for counter examples file in the wrong directory #190

Closed sashaafm closed 3 years ago

sashaafm commented 3 years ago

Hello @alfert, I've got an umbrella app with several child apps. Two of those child apps have been using propcheck for quite some time successfully. Now, I've added a third child app with propcheck, but I keep getting this error when attempting to run mix test from the umbrella's root directory:

15:01:05.941 [info]  Application propcheck exited: PropCheck.App.start(:normal, []) returned an error: shutdown: failed to start child: PropCheck.CounterStrike
    ** (EXIT) an exception was raised:
        ** (MatchError) no match of right hand side value: {:error, {:file_error, '/app/server/apps/new_child_app/_build/propcheck.ctex', :enoent}}
            (propcheck 1.3.0) lib/counterstrike.ex:53: PropCheck.CounterStrike.init/1
            (stdlib 3.14.1) gen_server.erl:417: :gen_server.init_it/2
            (stdlib 3.14.1) gen_server.erl:385: :gen_server.init_it/6
            (stdlib 3.14.1) proc_lib.erl:226: :proc_lib.init_p_do_apply/3

This directory is wrong as the counter examples file is being stored in /app/server/_build/propcheck.ctex as expected in an umbrella app.

I've attempted to solve this by adding: propcheck: [counter_examples: "../../_build/propcheck.ctex"] to my new child app's MixProject.project/0 function, but I get the same result. My other child apps don't even require this configuration to test properly from the umbrella's root.

Furthermore, I've done the usual cleaning deps and _build directories which did not help. I'm not sure if I'm missing something or if this is some kind of problem in propcheck itself.

Any hints would be helpful! Thanks!

evnu commented 3 years ago

I guess you already checked that, but is :build_path set correctly in the mix.exs of your new child app?

sashaafm commented 3 years ago

@evnu yes, I've already tripled checked and even copied those configs from the other child apps 😕

sashaafm commented 3 years ago

@evnu I'm quite sad to say that this issue is precisely the same as this https://github.com/alfert/propcheck/issues/115 which I had opened over an year ago. This error messages are coming from a cluster test with several nodes. In that older issue I had used Erlang's slave API but now I'm using the :local_cluster testing library which probably uses that API as well.

The reasons are the same as in that older issue. Slave nodes probably don't inherit the config from the mix.exs file and therefore instead of using the relative ../../build path are using the current directory path.

As stated in that other issue, I believe this would be solved by having the counter examples file be defined via application configuration (which is possible to set in the slave nodes). The :local_cluster library further corroborates this in this snippet from the README:

LocalCluster.start_nodes(:spawn, 1, [
  environment: [
    my_app: [
      port: 9999
    ]
  ]
])

I'd like to request if we could reopen that older PR and merge it? Sorry to bring the same issue again which we didn't get around to solving last time.

alfert commented 3 years ago

This makes absolutely sense.

alfert commented 3 years ago

@sashaafm could you please check if new master fixes your problem?

sashaafm commented 3 years ago

@alfert and @evnu I've used the branch from here: https://github.com/evnu/propcheck/tree/115-configure-counterexamples-path and upon adding the configuration to the LocalCluster in my setup_all like so:

    nodes = LocalCluster.start_nodes(:spawn, 3, [
      applications: [@app],
      environment: [
        propcheck: [
          counter_examples: "../../_build/propcheck.ctex"
        ]
      ]
    ])

I confirm it now works and my tests pass without those error messages from before 🙂

sashaafm commented 3 years ago

@alfert will this warrant a new release to have the Mixfile point to in Hex?

starbelly commented 3 years ago

@alfert A release would be very appreciated. It seems like a project I'm working on can not configure the counter examples file if propcheck is set as a test only dependency 🙏

alfert commented 3 years ago

@starbelly, @sashaafm Yes, we can do a new release. But what is required such that you will benefit from that release? I would go for having PropEr still in 1.3 as a fix dependency, which of course will bite you if you require OTP 23 support for stack traces.

Or do we need to provide an (additional?) release which uses a specific version of Proper (via git sha)? Or is that something you can do by yourself as an overriding dependency in your project's Mixfile and we should document this in the README explicitly?

starbelly commented 3 years ago

@alfert yes, precisely, if I follow you correctly. 1.3.1 makes sense to me. If we need to override proper we can do that in mix.

sashaafm commented 3 years ago

I agree a 1.3.1 release makes sense as these are non breaking changes and no API modifications. There we may point to it from Mix

alfert commented 3 years ago

Folks, releasing a 1.3.1 is not easy, since there is no planned release activities for PropEr. This means that we have to stick to PropEr 1.3.0 from hex, which is 3 years old and does not play well with OPT 23ff regarding stack traces. Also the new API changes from Proper which are already used in PropCheck's master branch will break if we do a release directly from master.

So, I prepared the following: A release branch for 1.3.1, where we stick to PropEr 1.3 and cherrypicked #115 . I will do a 1.3.1-rc1 release for upload in Hex. I hope that helps.

alfert commented 3 years ago

I close this bug, since PropCheck 1.4 with #202 is out and I did not receive any signal of bugs.