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

VAGOV-5767: Add WEB tests to CMS test suite. #547

Closed
wants to merge 11 commits into from

Conversation

jonpugh
Copy link
Contributor

@jonpugh jonpugh commented Aug 26, 2019

This PR adds the new test va/web/unit.

It kicks off the WEB unit tests after the WEB build and Behat tests run.

this PR is only going to add this test. Getting the full WEB test suite to pass requires further work.

Background

Environment variables and CLI options affect the build:
NODE_ENV=prod
--buildtype=VAGOVPROD will cause the build to fail if there are bad links.

So... Do we want the build to fail if the link checker fails? because right now that would block a merge! Who would this be a question for, @swirtSJW ?

@jonpugh
Copy link
Contributor Author

jonpugh commented Aug 26, 2019

First run for me results in a looping "Connection refused", presumably from the Mock API server.

@swirtSJW
Copy link
Contributor

@jonpugh That is an interesting find. Generally I am opposed to hamstringing development by content which is unrelated to development. If there were a way to check whether the links were failing because of code, that would be one thing, but making all development PR's fail because an editor enters a bad link or menu entry is making a dependency that should not exist.

@jonpugh
Copy link
Contributor Author

jonpugh commented Aug 27, 2019

Making all development PR's fail because an editor enters a bad link or menu entry is making a dependency that should not exist.

Oh, totally! That link checker fires about 100% of the time in my experience, so, we'll have to find a happy medium.

From my conversations with @ncksllvn, I think the other part that this change affects is the accessibility tests: With this new flag an a11y test fail will fail the build.

@jonpugh
Copy link
Contributor Author

jonpugh commented Aug 27, 2019

I'm not planning on spending much more time on this today.

I think I will roll back this PR to just get va/web/tests/unit passing

…ad, and add a separate v:w:b:prod command to keep the flag for later use.
swirtSJW
swirtSJW previously approved these changes Aug 27, 2019
Copy link
Contributor

@swirtSJW swirtSJW left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed but have not tested yet. If this does not fail our build based on FE tests of links and accessibility, I am good with it.

@jonpugh jonpugh changed the title VAGOV-5767: Add WEB tests to CMS test suite. VAGOV-5767: Add a single WEB test to CMS test suite. Aug 28, 2019
@ElijahLynn
Copy link
Contributor

I changed the base branch from develop to master as we switched our CD to be based on the GitHub Flow model (as opposed to Git Flow) and develop is now deprecated (#617). It may be best instead of fixing conflicts to rebase/replay your commits on top of current master:

git fetch --all
git rebase origin/master
git push --force

@jonpugh
Copy link
Contributor Author

jonpugh commented Feb 7, 2020

I want to bring this back once we have the Rebuild Web uI.

@swirtSJW
Copy link
Contributor

Blowing the dust off this. @jonpugh Does this do something we aren't already doing or can we close it?

@swirtSJW
Copy link
Contributor

Closing this as unresponsive. Re-open if needed.

@swirtSJW swirtSJW closed this Mar 23, 2020
@ElijahLynn ElijahLynn deleted the VAGOV-5767-web-tests branch November 20, 2020 19:27
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.

None yet

4 participants