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

#7324 Ensure Open Types tests are run (gradle) #7327

Merged
merged 12 commits into from
Mar 7, 2023
Merged

Conversation

planetf1
Copy link
Member

@planetf1 planetf1 commented Jan 26, 2023

Description

Ensures open types FVT tests run as part of gradle build

Related Issue(s)

See #7374

Testing

  • Open types fvt test source files created ok
  • Test launches ok
  • Cleanup code removes files afterwards

Note, this current fails as the actual tests have issues:

Screenshot 2023-01-26 at 18 55 39

The changes to the scripts appear good & ready for merge when we have fixed the tests.

Release Notes & Documentation

Additional notes

Once #7325 is merged, I will merge or rebase, as this PR currently also contains that fix - needed to validate

FYI @davidradl

@planetf1 planetf1 marked this pull request as ready for review January 27, 2023 12:02
@planetf1
Copy link
Member Author

planetf1 commented Jan 27, 2023

I notice codeQL is failing with lots of 'notes' (minor level issues) due to the generated code.

These can be seen at https://github.com/odpi/egeria/security/code-scanning?query=pr%3A7327+tool%3ACodeQL+is%3Aopen+sort%3Acreated-desc

It might be useful to fix the generator to build cleaner code (I suspect it's a small number of issues in the template)

Directories can't be excluded from the codeQL scan itself, the only control is by not-building certain modules.

We might be able to add properties, but this will change how users need to exclude tests (-x tests) -- the issue here is that the FVT generated code (which is a compile task, not test) will still run. A fix here will need some further refactoring ...

So probably easiest to take a look at the warnings.

Other alternatives

  • Switch off the 'note' level warnings from codeQL (since we do now have sonatype lift)
  • add a new custom property (ie ./gradle build -Pskip.tests and then use an ifOnly in the FVT tasks to skip FVTs

Still looking to see how I can code a condition based on the absence of other tests (to leverage the -x tests)

@planetf1
Copy link
Member Author

^ fyi @davidradl

@planetf1
Copy link
Member Author

planetf1 commented Feb 2, 2023

@davidradl Do you want to do anything with the code template?

Signed-off-by: Nigel Jones <[email protected]>
@planetf1 planetf1 closed this Mar 2, 2023
@planetf1 planetf1 reopened this Mar 2, 2023
@planetf1
Copy link
Member Author

planetf1 commented Mar 7, 2023

I'm trying to get these build issues integrated before some refactoring needed to fix connectors.
I think I can remove the open types test from the build - for the purposes of codeQL only.
This should fix the large number of observations.
If the build is good I will merge

@planetf1
Copy link
Member Author

planetf1 commented Mar 7, 2023

  • codeQL build still failing (the build itself is clean) - even without explicitly compiling the open types code. - suspect we are very marginal, as we're also seeing this now with spring3

@planetf1
Copy link
Member Author

planetf1 commented Mar 7, 2023

rules evaluation just seems to stop.

github/codeql#7294 seems to confirm that @SuppressWarnings is not respected by codeQL. However that issue does also point to a way of filtering the sarif file before analysis.

This may be what is needed to fix codeQL. However at this point I think it's clear this is a codeql specific issue. It's fragile.

I therefore plan on merging this anyway & raising a new issue to track codeQL issues

@planetf1 planetf1 merged commit 5789589 into odpi:main Mar 7, 2023
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