framework icon indicating copy to clipboard operation
framework copied to clipboard

Remove early return on `startup.cpp` when testing is enabled

Open lobis opened this issue 3 years ago • 7 comments

Originally startup.cpp caused problems with unit testing and to fix this we added an early return if testing was enabled:

#ifdef REST_TESTING_ENABLED
        return;
#endif

This doesn't seem to affect the running of tests, but makes the framework behave differently when testing is enabled, this means that users should not use the framework with tests compiled to produced results etc. and only use it to run the tests, then compile it again without tests. This is also true for the pipeline where in order to run tests the framework is compiled twice.

There should be a way to modify this file so that tests can be run and also the file keeps working as intended, so that we can avoid recompiling twice.

lobis avatar Mar 28 '22 07:03 lobis

If returned, the global variables REST_COMMIT/REST_PATH/REST_USER are not set. It will make REST behave differently. I think we should set these variables manually before early return under testing condition.

#ifdef REST_TESTING_ENABLED
    REST_COMMIT = xxxx;
    REST_PATH = xxxx;
    REST_USER = xxxx;
    REST_USER_PATH = xxxx;
   return;
#endif

What are the anticipated values of these variables?

nkx111 avatar Mar 28 '22 17:03 nkx111

I am not familiar with ctest. Seems that we can input c++ definitions to the code during the test. Then maybe it is also possible to do something like:

#ifdef REST_TESTING_ENABLED
    REST_COMMIT = REST_TESTING_VAR_COMMIT;
    REST_PATH = REST_TESTING_VAR_PATH;
    REST_USER = REST_TESTING_USER;
#endif

where REST_TESTING_XXX are all the imported c++ definitions from the test system.

nkx111 avatar Mar 28 '22 17:03 nkx111

If returned, the global variables REST_COMMIT/REST_PATH/REST_USER are not set. It will make REST behave differently. I think we should set these variables manually before early return under testing condition.

#ifdef REST_TESTING_ENABLED
    REST_COMMIT = xxxx;
    REST_PATH = xxxx;
    REST_USER = xxxx;
    REST_USER_PATH = xxxx;
   return;
#endif

What are the anticipated values of these variables?

I am not sure if I follow. The early return is a dirty fix I made in order to avoid tests failing, I think it should be possible to modify this class to remove the early return logic and make it work both during testing and not testing.

lobis avatar Mar 28 '22 21:03 lobis

I am not sure if I follow. The early return is a dirty fix I made in order to avoid tests failing, I think it should be possible to modify this class to remove the early return logic and make it work both during testing and not testing.

I was saying that the possible modification is like above. We can do the modification if someone specifies the REST_PATH, etc. values for testing environment.

nkx111 avatar Mar 28 '22 22:03 nkx111

I am not sure if I follow. The early return is a dirty fix I made in order to avoid tests failing, I think it should be possible to modify this class to remove the early return logic and make it work both during testing and not testing.

I was saying that the possible modification is like above. We can do the modification if someone specifies the REST_PATH, etc. values for testing environment.

But wouldn't these values such as REST_PATH be already in the environment variables?

lobis avatar Mar 29 '22 11:03 lobis

I am not sure if I follow. The early return is a dirty fix I made in order to avoid tests failing, I think it should be possible to modify this class to remove the early return logic and make it work both during testing and not testing.

I was saying that the possible modification is like above. We can do the modification if someone specifies the REST_PATH, etc. values for testing environment.

But wouldn't these values such as REST_PATH be already in the environment variables?

I guess not. REST_PATH is set only after calling thisREST.sh. Maybe the test system didn't do that.

nkx111 avatar Mar 29 '22 17:03 nkx111

I am not sure if I follow. The early return is a dirty fix I made in order to avoid tests failing, I think it should be possible to modify this class to remove the early return logic and make it work both during testing and not testing.

I was saying that the possible modification is like above. We can do the modification if someone specifies the REST_PATH, etc. values for testing environment.

But wouldn't these values such as REST_PATH be already in the environment variables?

I guess not. REST_PATH is set only after calling thisREST.sh. Maybe the test system didn't do that.

When you run ctest you should have the same env variables that when you run i.e. restManager. It will fail even after running source thisREST.sh

lobis avatar Mar 29 '22 22:03 lobis