feat: add enforce support for pipeline schedule

[amimas]

This change adds enforce support to pipeline schedule configuration. By adding enforce: true, unconfigured schedules can be deleted. This can help keep the gitlabform config and project's pipeline schedule's in sync.

closes #539

[amimas]

@gdubicki - It seems the tests are currently setup to be dependent on earlier test case's results. That's why this assertion looks odd to me. I expected each test case to be self-contained and not affecting other tests. I'm not sure if this was done intentionally. Thought I'd mention.

[gdubicki]

This is a result of the fact that we are often using fixtures that have a pytest scope higher than "function". Particularly, the fixture for a project has scope "class", so the schedules created in one method of a test class such as TestSchedules will in fact be retained in the next test method.

To fix that we would have to use "function" scope for an almost perfect separation of each test but we are are talking about tests on a real-world GitLab instance where creating and deleting projects in GitLab is pretty slow.

I just did a test of how this would work out in practice for test_schedules.py:

Code as-is, with project and group having scope "class":

1st run - 7.708s
2nd run - 6.280s
3rd run - 7.390s

Code with project and group scope changed to "function":

1st run - 14.556s
2nd run - 18.286s
3rd run - 18.335s

So the tests time effectively increases 2-2.5 times.

The total current time of running tests in GitHub Actions is ~3 minutes for standard and ~1.5 minutes for premium ones, so including the ~4 minutes of GitLab provisioning time the whole pipeline time would increase from ~7 minutes to ~10 minutes...

Because of that we are now using 2 ways of dealing with the issue of tests affecting each other, only where it affects the tests:

a) use "function" scoped fixtures on specific single tests - see the uses of project_for_function fixture,
b) make the assertions about the initial state more relaxed - see https://github.com/gitlabform/gitlabform/blob/main/tests/acceptance/standard/test_members.py#L12-L12 as an example.

Of course this is not perfect, but it's the best compromise between correctness and performance of the tests that I was able to develop at the time. If you think that we should make changes then feel free to share your ideas, @amimas. :)

[amimas]

Thanks @gdubicki for the details and the performance tests. I also realized this came down to fixture scope after looking into it a bit, but didn't have time to learn what changes are needed if the scope is changed.

I'm starting to think what you've got right now actually makes sense. These are not mocked unit tests. As you said, we're running these against a real GitLab instance. Also not sure if there's a real need for running only individual test as opposed to running all tests in a single file. That would probably make the testing and development faster, but is it worth it? So, it is a balance between isolated tests vs performance of entire test suites.

I don't have any other ideas at the moment. Being new to the code base, it was a bit confusing at the beginning. I had to go through existing test cases and then figure out if the assertions in the new test case makes sense. Maybe what might help others is if we have additional sections in the contributing docs about general setup/expectations of acceptance tests. I have only played with 1 test file through this PR so far. If I can think of any improvements to the docs/development guide in future, I'll open new PR accordingly.

[gdubicki]

I agree, we should at least document it. I'll try to do it soon.

[codecov]

Codecov Report

Merging #561 (2f6c4b8) into main (04deee9) will increase coverage by 0.27%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #561      +/-   ##
==========================================
+ Coverage   82.73%   83.00%   +0.27%     
==========================================
  Files          70       70              
  Lines        2659     2672      +13     
==========================================
+ Hits         2200     2218      +18     
+ Misses        459      454       -5     

Impacted Files	Coverage Δ
...tlabform/processors/project/schedules_processor.py	93.84% <100.00%> (+1.53%)	⬆️

... and 5 files with indirect coverage changes

[amimas]

Just realized that type checking is passing in the CI but it's failing on my local machine.

(venv) ➜  gitlabform git:(issue-539) mypy . || true
gitlabform/processors/project/schedules_processor.py:18: error: Item "None" of "Optional[Any]" has no attribute "pop"  [union-attr]
gitlabform/processors/project/schedules_processor.py:25: error: Argument 1 to "sorted" has incompatible type "Optional[Any]"; expected "Iterable[Any]"  [arg-type]
gitlabform/processors/project/schedules_processor.py:28: error: Value of type "Optional[Any]" is not indexable  [index]
gitlabform/processors/project/schedules_processor.py:85: error: Unsupported right operand type for in ("Optional[Any]")  [operator]
Found 4 errors in 1 file (checked 132 source files)

[gdubicki]

Just realized that type checking is passing in the CI but it's failing on my local machine. (...)

I am having the same issue. :/ It's not the first strange behaviour of mypy that I have observed but frankly to me mypy failures are not serious enough and at the same time it was difficult to troubleshoot so I gave up. But if you want to try to get to the bottom of this, feel free! :)

[amimas]

Just realized that type checking is passing in the CI but it's failing on my local machine. (...)

I am having the same issue. :/ It's not the first strange behaviour of mypy that I have observed but frankly to me mypy failures are not serious enough and at the same time it was difficult to troubleshoot so I gave up. But if you want to try to get to the bottom of this, feel free! :)

If you think mypy issues are not serious enough, that's fine. I'm new to this as well :). What bugs me though is the behaviour difference between my local machine vs CI. I might look into that a bit, unless you already know possible reasons.

[amimas]

Just realized that type checking is passing in the CI but it's failing on my local machine. (...)

I am having the same issue. :/ It's not the first strange behaviour of mypy that I have observed but frankly to me mypy failures are not serious enough and at the same time it was difficult to troubleshoot so I gave up. But if you want to try to get to the bottom of this, feel free! :)

If you think mypy issues are not serious enough, that's fine. I'm new to this as well :). What bugs me though is the behaviour difference between my local machine vs CI. I might look into that a bit, unless you already know possible reasons.

I couldn't figure out the discrepancy between CI and my local machine. But, I've made a small tweak to the code and mypy wasn't reporting issues on my local machine anymore.

[gdubicki]

LGTM now!