Skip to content

Conversation

@MarDiehl
Copy link
Contributor

GitHub CI now has singularity, use it to test python 3.7, 3.8, and 3.9.

GitHub CI now has singularity.
@vsoch
Copy link
Member

vsoch commented Sep 30, 2021

This is fantastic that you got this working! If all is good we can cancel / stop the tests on CircleCI - there isn't good reason to maintain two testing setups.


steps:
- uses: actions/checkout@v2
- uses: eWaterCycle/setup-singularity@v6
Copy link
Member

Choose a reason for hiding this comment

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

This is ready to go pending one request - I'm a bit conservative about using actions that I don't control (and that aren't under GitHub's umbrella). Could you please update this to be the commit associated with the release and then comment for the tag? e.g.,:

Suggested change
- uses: eWaterCycle/setup-singularity@v6
- uses: eWaterCycle/setup-singularity@1631b12cf3878381179be0eab9624219bc12979e # v6 release

Looked up: eWaterCycle/setup-singularity@1631b12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

better be careful with actions not under control of GitHub or any other
trusted source
@vsoch
Copy link
Member

vsoch commented Oct 1, 2021

Two quick questions - first I see this popped up in a few of the tests here:

ERROR ['Error for command "stop": no instance found\n\nUsage:\n  singularity [global options...] instance stop [stop options...] [instance]\n\nRun \'singularity --help\' for more detailed usage information.\n'] : return code 1

It didn't fail the test, but I suspect either the instances are being controlled across the matrix, or something is in parallel that should not be. I don't think 3.9 has the error https://github.com/singularityhub/singularity-cli/pull/183/checks?check_run_id=3768796742 so I suspect 3.9 stopped the instances for the other two, and I don't think we want to do that. I also noticed that your command to run tests is:

COLUMNS=256 python3 setup.py pytest

and if this could be possibly related? What does COLUMNS do (the output does not look the same as it does in circle, see here https://app.circleci.com/pipelines/github/singularityhub/singularity-cli/341/workflows/db86650b-1e64-4c15-9132-d2757a242194/jobs/1209). Would it make sense to use the same test command as we do there? E.g.,

pytest ~/repo/spython

Would basically be running pytest on the spython directory, so probably just pytest spython here.

So to summarize:

  1. Let's try the same way to run tests as in circle so the output looks a bit nicer
  2. Let's have the instance names be specific to the python version (just grab it from a sys import in the test) so we don't have this interaction between matrix entries.

@vsoch
Copy link
Member

vsoch commented Oct 1, 2021

@MarDiehl to clarify what I think the issue is - when you manually run a service in a matrix, all the jobs in the matrix see the same services. So if we do a singularity instance stop for Python 3.8, it's also going to hit the others. The way to avoid this is to name the instances, and stop them by name instead of using stop all.

@MarDiehl
Copy link
Contributor Author

MarDiehl commented Oct 1, 2021

done

@MarDiehl
Copy link
Contributor Author

MarDiehl commented Oct 1, 2021

@MarDiehl to clarify what I think the issue is - when you manually run a service in a matrix, all the jobs in the matrix see the same services. So if we do a singularity instance stop for Python 3.8, it's also going to hit the others. The way to avoid this is to name the instances, and stop them by name instead of using stop all.

This is a little bit above my knowledge. I would expect it comes from Client.instance_stopall() in test_instances.py. But I would be surprised if anything is shared between the python variants. To me it looks as if they are completely isolated.

@vsoch
Copy link
Member

vsoch commented Oct 1, 2021

I’m hypothesizing that because I see the error only for 2/3. If it’s ok with you, I can start from your branch and test a few things to figure this out over the weekend!

@MarDiehl
Copy link
Contributor Author

MarDiehl commented Oct 1, 2021

I’m hypothesizing that because I see the error only for 2/3. If it’s ok with you, I can start from your branch and test a few things to figure this out over the weekend!

sure, I think I've done everything I can.

@vsoch
Copy link
Member

vsoch commented Oct 1, 2021

Yes and you've done great! I am definitely happy to take over from here. Will give you an update with what I learn.

@vsoch vsoch mentioned this pull request Oct 2, 2021
@vsoch
Copy link
Member

vsoch commented Oct 2, 2021

Included in #185 - thank you @MarDiehl !

@vsoch vsoch closed this Oct 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants