Test paas-app-charmer changes in discourse-gateway #330
Conversation
Hopefully the linter is good...
| strategy: | ||
| max-parallel: 1 | ||
| matrix: | ||
| charm-dir: | ||
| - | ||
| - charm |
There was a problem hiding this comment.
I'm hesitant to add complexity like this to the e2e tests, especially since we have to write them in bash and that when it is the charm case we also actually change from metadata to charmcraft. 2 options i can think of:
- Add another stage to the tests which change from metadata to charmcraft and inside a directory
- Add a separate test file
I would also run this in reconclide rather than conflict since we're not really testing a conflict.
Probably we can go with adding another job to reconcile which has 1 documentation file inside a directory and making use of charmcraft rather than metadata and just run the create action on that and leave it at that
There was a problem hiding this comment.
I would also run this in reconclide rather than conflict since we're not really testing a conflict.
I'm fine with either, but if we go with 1) I would put this in the conflict tests. End2End tests test a number of things and integrations beside conflicts, so that should be totally fine. The reason why I would not put this in the reconcile is that reconcile is the test where we do update production Discourse (to make sure the integration works correctly) which we don't run in scheduled fashion to avoid cluttering Discourse too much. Anything we put there is somewhat less checked.
So by default, we should place more things on pulling tests (such as the conflict ones) that we run often and don't clutter Discourse.
There was a problem hiding this comment.
Sounds good, perhaps we can rename this file
There was a problem hiding this comment.
Yes, I'm happy with this, I also find it somewhat confusing. We could just go for "Pulling Test" and "Pushing Test". If we will start splitting tests further we could revisit this
There was a problem hiding this comment.
Changes look sane. I have raised a comment on @jdkandersson but once that is settled I'm happy to approve.
Only one note, we should probably raise dedicated PR for updating the gatekeeper code and also possibly find a better automation for doing this.
I am working on the PR for updating the gatekeeper code in canonical/discourse-gatekeeper#244 and #466. I will left this PR open until that is done. Thanks for the review! |
|
I close this PR and open a simpler one in another PR with just the new test |
Problem
Following canonical/discourse-gatekeeper#238, the charm files (charmcraft.yaml, metadata.yaml and the docs directory) can be in a directory that is not in the repository base directory. This directory is specified by the github action input variable
charm_dirindiscourse-gateway. Besides, to get information from the charm, if themetadata.yamlfile is not in the project, thecharmcraft.yamlfile will be used.Context
Using a customised directory instead of the base one and using only the
charmcraft.yamlfile are requirements for PAAS app charmer applications to work correctly. Besides, the use ofcharmcraft.yamlis the recommended way of specifying the name of the charm and the link to the doc for every project (see https://juju.is/docs/sdk/charmcraft-yaml).Solution
See canonical/discourse-gatekeeper#238.
Testing
A matrix strategy for workflows
e2e_conflicts.yamlande2e_test.yamlto test using the the base directory and thecharmdirectory as charm_dir inputs.In the workflow
e2e_conflicts.yamlwhen usingcharmascharm_dir, thecharmcraft.yamlfile will be used instead of themetadata.yamlfile.Release Notes
New input parameter
charm_dirfordiscourse-gatekeeper accepts, that allows specifying where the charm files and directories are located it they are not in the base directory of the repository.If the file
metadata.yamldoes not exist, the filecharmcraft.yamlis used to get the charm name and the link to the documentation.