search

docker-archive / oscalkit

NIST OSCAL SDK and CLI
https://docker.github.io/oscalkit/
Other
36 stars 23 forks source link

Fix failing test in test/main.go #54

Closed anweiss closed 5 years ago

anweiss commented 5 years ago

Test fails for test/main.go:

test/main.go:13:14: undefined: oscalkit.ApplicableControls

@farhan-khan30 can you address this?

Supports #10.

anweiss commented 5 years ago

@minhaj10p could you help with this one? We need to exclude this test since it's dependent on the generated code in templates/catalog.go

asadullah-yousuf-10p commented 5 years ago

@anweiss looks like this issue is related to #35 We do need to exclude this folder and use bash scripts to run this test instead. @farhan-khan30 what do you think?

farhankamalkhan-10p commented 5 years ago

@anweiss #35 and this current issue are both occurring because the class being tested currently does not exist. As mentioned in the readme of this test, i.e. "NOTE: This is a test so output.go (generated using oscalkit cli) should already exist when running this test.", cli/main.go generate has to be run before the test can be executed as I can not test something that does not exist. Since the test imports the generated code, the test will not compile without output.go existing. The solution to this could be to 1) Exclude the test from CircleCI 2) Create a bash test target that runs the actual test by running cli/main.go generate, running the actual test, and removing output.go.

anweiss commented 5 years ago

@farhan-khan30 let's see if we can go with option 2 that you've proposed, so that we can still run the test against the generated code, rather than excluding it altogether.

farhankamalkhan-10p commented 5 years ago

@anweiss as suggested in option 2 created PR https://github.com/docker/oscalkit/pull/72. I have created a bash script that will run cli/main.go generate code before running the test. I have also made some changes to the Makefile so that make test will run the test with the NIST profiles before running all the other tests. This test does not handle the profile in profile import scenario for now.

anweiss commented 5 years ago

Closing for now. Can revisit other issues separately.