Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add vsoch + add tests? #2

Merged
merged 4 commits into from
Dec 6, 2021
Merged

Conversation

ngam
Copy link
Contributor

@ngam ngam commented Dec 6, 2021

@vsoch (as per discussion in singularityhub/singularity-hpc#464) I had some of these tests in earlier iterations, but there were issues here and there. I am happy to add them again in this pr to assess how best to approach this.

    ## possible tests to run to ensure qc?
#     - pytest shpc/tests/test_settings.py  # [not win]
#     ## ignore linux test because test returns verbose value
# #     - pytest shpc/tests/test_utils.py  # [not linux64]
#     - pytest shpc/tests/test_container_config.py  # [not win] 
#     ## ignore othere tests because they trigger docker/singularity/podman

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@vsoch I had some of these tests in earlier iterations, but there were issues here and there. I am happy to add them again in this pr to assess how best to approach this.

    ## possible tests to run to ensure qc?
#     - pytest shpc/tests/test_settings.py  # [not win]
#     ## ignore linux test because test returns verbose value
# #     - pytest shpc/tests/test_utils.py  # [not linux64]
#     - pytest shpc/tests/test_container_config.py  # [not win] 
#     ## ignore othere tests because they trigger docker/singularity/podman
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@ngam
Copy link
Contributor Author

ngam commented Dec 6, 2021

@conda-forge-admin, please rerender

@ngam ngam changed the title Add vsoch Add vsoch + add tests Dec 6, 2021
@vsoch
Copy link
Contributor

vsoch commented Dec 6, 2021

@ngam with respect to testing, my thinking for conda recipes is that the minimal test is just to do shpc --version (or version). Personally speaking I think shpc (at its repo) should be the main test runner - especially the tests here that require container technologies / lmod / modules that would need to be installed. We can't easily do that here.

@ngam
Copy link
Contributor Author

ngam commented Dec 6, 2021

@ngam with respect to testing, my thinking for conda recipes is that the minimal test is just to do shpc --version (or version). Personally speaking I think shpc (at its repo) should be the main test runner - especially the tests here that require container technologies / lmod / modules that would need to be installed. We can't easily do that here.

Yes, that was exactly my rationale, see initial pr and discussion (with myself) here: conda-forge/staged-recipes#17085

@ngam
Copy link
Contributor Author

ngam commented Dec 6, 2021

And especially that the build is pretty similar, etc. testing here wouldn't really be all that valuable. We usually like to do rigorous tests for more involved builds --- did you know we build singularity/ce itself here too? I am also adding apptainer soon to stay up to date with the drama 🤦

https://github.com/conda-forge/singularity-feedstock
https://github.com/conda-forge/singularityce-feedstock

@ngam
Copy link
Contributor Author

ngam commented Dec 6, 2021

@vsoch, are you cool then with me merging this or would you like some more time to see if you'd like anything changed? Once merged, you will have the same access as me 😄

@ngam ngam changed the title Add vsoch + add tests Add vsoch + add tests? Dec 6, 2021
@vsoch
Copy link
Contributor

vsoch commented Dec 6, 2021

I think it looks great - merge away!

@ngam ngam added the automerge Merge the PR when CI passes label Dec 6, 2021
@github-actions github-actions bot merged commit a66dcc3 into conda-forge:master Dec 6, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2021

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

  • linter: passed
  • azure: passed

Thus the PR was passing and merged! Have a great day!

@ngam ngam deleted the add_vsoch branch December 6, 2021 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge the PR when CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants