Skip to content

Investigate unexpected licence check failures.#373

Closed
pp-mo wants to merge 3 commits intoSciTools:mainfrom
pp-mo:licence_check
Closed

Investigate unexpected licence check failures.#373
pp-mo wants to merge 3 commits intoSciTools:mainfrom
pp-mo:licence_check

Conversation

@pp-mo
Copy link
Member

@pp-mo pp-mo commented Jan 12, 2024

I can't any difference between the licence headers of test_grid_definition_template_40.py and test_grid_definition_template_140.py, but the latter is failing.

The header check only runs on files changed in the current commit (or something like).
So we shall see whether this fails on the old file (previously apparently OK) or not ...

@pp-mo
Copy link
Member Author

pp-mo commented Jan 12, 2024

Weirder and weirder...

In this PR, it still complains about the file "..._140.py".
But that file is not changed, so should not get tested here IIUC.

And still I can detect no difference in the header lines between this and the "..._40.py" one.

@pp-mo
Copy link
Member Author

pp-mo commented Jan 12, 2024

The latest results do seem to show that there is not anything wrong in the "test_..._140.py" file,
since it does not appear alongside "test_..._20.py" in the list of failing ones shown in the test output.

So far, I am stumped by this. There must be something wrong with how the test is working.
@trexfeathers can you make any sense of what might be happening here ?

@pp-mo pp-mo changed the title Tweak to prompt licence check on exiting file. Tweak to prompt licence check on existing file. Jan 12, 2024
@pp-mo pp-mo changed the title Tweak to prompt licence check on existing file. Investigate unexpected licence check failures. Jan 12, 2024
@trexfeathers
Copy link
Contributor

The latest results do seem to show that there is not anything wrong in the "test_...140.py" file, since it does not appear alongside "test..._20.py" in the list of failing ones shown in the test output.

So far, I am stumped by this. There must be something wrong with how the test is working. @trexfeathers can you make any sense of what might be happening here ?

I think you're looking in the wrong place. The failure is in test_grid_definition_template_140.py within save_rules/ (not load_convert/). The one in save rules does indeed have the wrong licence header:

# Copyright iris-grib contributors
#
# This file is part of iris-grib and is released under the LGPL license.
# See COPYING and COPYING.LESSER in the root of the repository for full
# licensing details.

This got merged unnoticed because #343 was raised before #359, but was merged AFTER it. We have got used to the excellent functionality of GHA, which gives us CI against a simulated merge, whereas Cirrus runs CI on the branch itself. This meant that #343 passed, because the licence header test on #343 was still asserting for the old header.

I assumed that the failures on #371 couldn't possibly be real, since I had not modified the file in question. But the failures were real, they just hadn't been noticed before. I'm probably going to run out of time to raise a fix PR this evening, but I'd be happy to merge one if you raised it.

@pp-mo
Copy link
Member Author

pp-mo commented Jan 12, 2024

Looking at the wrong file !
There are two files called test_grid_definition_template_140.py,
I have been looking at tests/unit/load_convert,
but the one that matter is in tests/unit/save_rules

@pp-mo pp-mo closed this Jan 12, 2024
@pp-mo pp-mo deleted the licence_check branch November 29, 2024 16:33
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