inviqa / harness-base-php

Base PHP harness for workspace https://github.com/my127/workspace
Other
4 stars 5 forks source link

Delete console container after quality tests #768

Closed g-foster2020 closed 1 year ago

g-foster2020 commented 1 year ago

Ensure console container is rebuilt after having dev dependencies installed for quality tests.

g-foster2020 commented 1 year ago

destroy is too destructive and acceptance tests need dev dependencies

I can't see how the acceptance tests can require the dev dependencies because this works?

andytson-inviqa commented 1 year ago

there's something more to it about these changes then, as obviously behat and co are dev dependencies and so would exist in console container

andytson-inviqa commented 1 year ago

you're missing a harness label so no harnesses were tested

g-foster2020 commented 1 year ago

Yes I can see run_project_quality_tests gets run twice. Before ws install and then during the acceptance tests. The issue was having it run before without cleaning up.

g-foster2020 commented 1 year ago

Yes I can see run_project_quality_tests gets run twice. Before ws install and then during the acceptance tests. The issue was having it run before without cleaning up.

sorry ignore this ... run for magento only during acceptance tests

g-foster2020 commented 1 year ago

This does seem to be the wrong solution.

The issue is that ws install didn't fail when it should have done, because the quality tests were run first and they installed dev dependencies which hid an issue.

I think it would be better to either remove the dev dependencies after they are needed or have a clean environment before running install_environment.

andytson-inviqa commented 1 year ago

Also the other thing is that project Jenkinsfile also has the dev dependencies installed behaviour. The test.sh is recreating those Jenkinsfiles steps in bash

g-foster2020 commented 1 year ago

It would be clearer to revert these changes and have a specific test_harness_install step.

g-foster2020 commented 1 year ago

This was a misguided solution due to lack of experience with the test script. PR will be closed and an issue opened for future consideration.