test(modchananalysis): test to check that nrnivmodl is run when required

[sanjayankur31]

See #165
See #216

[borismarin]

Great. I have also tested (locally) the case when nrnivmodl fails to compile (by introducing a syntax error into the mod file by hand), in which case we return 1 (maybe not the best error handling) - should our tests catch that as well?

[borismarin]

I am also foreseeing trouble with the hardcoded path for the compiled mechs. Seems to work fine for my machine (Linux x86_64) as well as the CI virtual machine - but I do not know what neuron does nowadays for other architectures. We could try and build the path using platform.processor(), but I am not sure nrnivmodl will always agree to that. Maybe we could have someone with a fancy new Apple silicon test it?

[sanjayankur31]

Great. I have also tested (locally) the case when nrnivmodl fails to compile (by introducing a syntax error into the mod file by hand), in which case we return 1 (maybe not the best error handling) - should our tests catch that as well?

Yeh, that'll be good to have as well---the more tests, the better!

I am also foreseeing trouble with the hardcoded path for the compiled mechs. Seems to work fine for my machine (Linux x86_64) as well as the CI virtual machine - but I do not know what neuron does nowadays for other architectures. We could try and build the path using platform.processor(), but I am not sure nrnivmodl will always agree to that. Maybe we could have someone with a fancy new Apple silicon test it?

Ah yeh. I think there are some bits in org.neuroml.export etc too that also need to be updated for this. nrnivmodl uses @host_cpu@ which is set to ${CMAKE_SYSTEM_PROCESSOR}:

https://github.com/neuronsimulator/nrn/blob/master/bin/nrnivmodl.in#L4

https://github.com/neuronsimulator/nrn/blob/master/cmake/ConfigFileSetting.cmake#L36

https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_PROCESSOR.html?highlight=cmake_system_processor

https://cmake.org/cmake/help/latest/variable/CMAKE_HOST_SYSTEM_PROCESSOR.html#variable:CMAKE_HOST_SYSTEM_PROCESSOR

I think platform.processor should be find to use---it's used in the platform.uname output, which we'd expect to be similar/same as uname?

I think @pgleeson did just recently get a fancy new macbook to test this out for us :P

[sanjayankur31]

I'll merge this and we can make more tweaks to the original PR, and continue the discussion etc. there too.