#980 Expand scope of isProblem.

[andreleblanc11]

Found a bug with the isProblem added from the previous merge request for Sundew fixes.

Also found another bug that wasn't affecting dissemination.

[andreleblanc11]

NOTE: This branch doesn't include unit tests as the branch is divergred from #1036

[github-actions]

Test Results

244 tests   235 ✅  20s ⏱️
  1 suites    8 💤
  1 files      1 ❌

For more details on these failures, see this check.

Results for commit d1a463e.

♻️ This comment has been updated with latest results.

[petersilva]

Is this trying to fix the BUFR thing? looks a lot like it.

[andreleblanc11]

No, this is actually unrelated to MetPX/Sundew#20.

This is me not having a proper testing infrastructure on DIRT and finding bugs later on while releasing code..

I have since corrected my testing infrastructure and this shouldn't happen again.

[andreleblanc11]

Should leverage getStation method from bulletin.py.

[petersilva]

are you going to do that, or you want this merged as-is?

[andreleblanc11]

I'll be doing that. I'll confirm once the PR is ready again.

[andreleblanc11]

Unit test failures unrelated to this PR. Ran gather/am tests locally and they all pass.

Auditing the changes overnight to see if all looks well.

[andreleblanc11]

sigh

the unit tests are failing because I've modified a method recently and if I change it back now while testing it will cause this branches' code to fail.

Will need to commit the correction after verifications are done

[petersilva]

yeah.. a challenge of unit tests is that they should be testing invariant, intended behaviour, and not stuff that just happens to be that way in the code as written.

[andreleblanc11]

The PR should be ready now and good to go. However... idk why my changes to bulletin.py are in my last commit but not in the change logs?

Anyways, a summary of what I've changed.

1. The initial problem was that the isProblem variable didn't have a broad enough scope when I first introduced it so I fixed it.
2. At the same time, it was noticed that the station logic from the gather routine was outdated so now we call getStation which will correctly get the station.
3. After adding the getStation call, I noticed a bug in the station mapping where we only checked for the first 2 chars which led to false positives. Now, we check if the whole 6 character header string matches if more then 2 chars a found in the bulletin. This is done because some bulletins, SI and SM bulletins, arrive only with a 2 character header so the 2 character check needs to remain for these.
4. For those same SM and SI bulletins, we need verifyHeader to have the full new header before it getting called. For this, I moved the bulletin reconstruct logic to its own method so I could call it before and after calling verifyHeader
5. I found another bug with the BBB bulletins in verifyHeader

[petersilva]

does not affect flow tests.. all changes confined to AM which is tested by @andreleblanc11 who confirms they're good.

Passes new unit tests as well.