Skip to content
This repository was archived by the owner on Aug 14, 2025. It is now read-only.

Conversation

@BenjaminRodenberg
Copy link
Member

system_testing.py currently does not allow to specify the SYSTEST_REMOTE via --docker-username. SYSTEST_REMOTE is set via a file tests/*/.env, where the remote SYSTEST_REMOTE = precice/is provided. This is hardcoded and causes problems, if one wants to run the systemtests on precice or adapter baseimages from a remote different than precice (e.g. benjaminrueth, which I'm using for development on my fork).

I'm not 100% sure about the reasons for specifying SYSTEST_REMOTE via the .env file. From my perspective this approach is mainly causing trouble, if I'm working on a fork. If somebody has a different perspective and knows a reason for specifying SYSTEST_REMOTE via the .env file, feedback would be highly appreciated.

What I tested (so far)

  • python3 system_testing.py --base Ubuntu1804.home -s fe-fe
  • python3 system_testing.py --base Ubuntu1804.home -s fe-fe --docker-username benjaminrueth
  • python3 system_testing.py --base Ubuntu1604.home -s bindings --docker-username precice
  • python3 system_testing.py --base Ubuntu1604.home -s bindings --docker-username benjaminrueth (does not work, since precice baseimage on benjaminrueth is outdated)
  • python3 system_testing.py --base Ubuntu1804.home -s bindings --docker-username precice
  • python3 system_testing.py --base Ubuntu1804.home -s bindings --docker-username benjaminrueth

@BenjaminRodenberg BenjaminRodenberg added the enhancement New feature or request label Feb 22, 2020
@BenjaminRodenberg BenjaminRodenberg self-assigned this Feb 22, 2020
@shkodm
Copy link
Contributor

shkodm commented Feb 23, 2020

Hey @BenjaminRueth .
I would not say it is a problem to have SYSTEST_REMOTE in the .env. file. It is just the default location where docker-compose looks up for the variables if they are not environment variables. In either case I would keep it there, you can simply specify configure export SYSTEST_REMOTE in run_compose in python scripts to configure it to point to you fork, etc. I did not do it, because noone requested it at the time :)
Having SYSTEST_REMOTE in .env. file also allows to cdinto test directory and run docker-compose up` without the need to run python scripts, which greatly simplified development and debugging. It is also makes things more explicit.

@shkodm shkodm self-requested a review February 23, 2020 11:11
@@ -1,4 +1,3 @@
OPENFOAM_TAG=latest
DEALII_TAG=latest
SYSTEST_REMOTE=precice/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line here and below could be kept, no need to delete it. It does not influence the possibility of repo specification above.

@BenjaminRodenberg
Copy link
Member Author

Hey @BenjaminRueth .
I would not say it is a problem to have SYSTEST_REMOTE in the .env. file. It is just the default location where docker-compose looks up for the variables if they are not environment variables. In either case I would keep it there, you can simply specify configure export SYSTEST_REMOTE in run_compose in python scripts to configure it to point to you fork, etc. I did not do it, because noone requested it at the time :)
Having SYSTEST_REMOTE in .env. file also allows to cdinto test directory and run docker-compose up` without the need to run python scripts, which greatly simplified development and debugging. It is also makes things more explicit.

Hi @shkodm! Thanks for the info. I did not know this. Just to make sure I understand: .env defines a default for SYSTEST_REMOTE, but I can overwrite this default (e.g. with a export SYSTEST_REMOTE called in a python script)?

In this case it sounds reasonable to leave the .env files as they are, but add the --docker-username as an optional input argument.

@shkodm
Copy link
Contributor

shkodm commented Feb 23, 2020

Just to make sure I understand: .env defines a default for SYSTEST_REMOTE, but I can overwrite this default (e.g. with a export SYSTEST_REMOTE called in a python script)?

Yes, exactly. From the docker docs:

Values present in the environment at runtime always override those defined inside the .env file. Similarly, values passed via command-line arguments take precedence as well.

Basically it is just a fallback if the variable could not be found anywhere

@BenjaminRodenberg BenjaminRodenberg marked this pull request as ready for review March 2, 2020 09:24
@BenjaminRodenberg BenjaminRodenberg requested a review from Eder-K March 2, 2020 09:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants