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

The DOM has changed, we must update #389

Closed
plocket opened this issue Nov 22, 2021 · 5 comments · Fixed by #391
Closed

The DOM has changed, we must update #389

plocket opened this issue Nov 22, 2021 · 5 comments · Fixed by #391
Assignees

Comments

@plocket
Copy link
Collaborator

plocket commented Nov 22, 2021

This will bump us to v3.

See SuffolkLITLab/docassemble-ALKilnSetup#59 for fields that need to be checked and possibly updated.

See SuffolkLITLab/docassemble-ALKilnSetup#160 for updating the DOM on those fields.

@plocket plocket self-assigned this Nov 22, 2021
@plocket
Copy link
Collaborator Author

plocket commented Nov 22, 2021

Observations:

@plocket
Copy link
Collaborator Author

plocket commented Nov 24, 2021

In https://github.com/plocket/docassemble-ALAutomatedTestingTests, we changed some of the multiple choice with none of the above questions to not have (remove) none of the above so that our tests can still pass those questions by as the were doing before the behavior change.

@plocket
Copy link
Collaborator Author

plocket commented Nov 30, 2021

Maybe page load isn't being checked for correctly anymore. Trying to figure out why tests keep working locally and failing on GitHub.

@plocket
Copy link
Collaborator Author

plocket commented Dec 2, 2021

[Context: Tests time out when the server is reloading. Most troublesome - the first test that is run always times out because of server reload from uploading the code for that test itself.]

Page is just not loading anymore. Jonathan says he's never seen a "ModuleNotFound" error. I swear to god it was a thing.

Anyway, that means that the page just hangs, the special timeout doesn't get triggered, and the step times out, then the next loop is triggered somehow and the interview gets loaded again.

Ideas

1. Guess at (only the first test's) load time

Use the full timeout to try to load the page for the first test. Repeat that 3 or 4 times. Maybe let the user know that's happening.

Pros:

  • it's quick and easy and we can do it now. Would need to allow a custom timeout to be set before initial page load, which we need to do anyway

Cons:

  • [The system timeout we set doesn't guarantee server is loaded in that amount of time, etc. The timeout is always somewhat of a guess as to when processes will finish.]
  • Only feasible for the first test load time unless we want to really increase the time it takes timeouts to fail all tests.
  • If we do this now and update to option 2 later, users will have to update to add the API key. We don't/won't have that many users, though, so maybe it wouldn't be a huge burden if we make the change a top priority.

2. API key

If we get the developer to put an API key in the secrets:

...the API for installing a package returns a code that can be passed to the package_update_status endpoint, which will return completed when the server has finished restarting. See https://docassemble.org/docs/api.html#package_update_status

Also, when polling that endpoint, I recommend using a short timeout and retry the API call in a loop. Depending on how busy the server is, a call to the package_update_status API may time out, so it is best to poll it in a loop, with the assumption that the server may not be able to respond to the status request while it is in the middle of restarting.

Pseudo code for pressing button or following a link in the middle of tests maybe (rationale - if another person causes a server restart in the middle of a test, it will create the same problem)

  1. Check if the server is busy
  2. Press button (or do any action?)
  3. Somehow check if it's trying to load a page (makes a request to the server? what kind?)
  4. If the page doesn't load in given time
  5. again check if server is busy
  6. if busy
    1. keep checking until server is not busy
  7. if not busy
    1. Fail

What about links to outside pages? I guess it doesn't do much harm to wait till the da server is free anyway.

Pros: More consistent, able to help at any time possibly
Cons:

  • More complicated setup for developers
  • We'd probably need to figure out how to log into an account using the API key instead of the username/password so the developer doesn't have to give all that info. That said, that could be a future simplification.
  • We'd need to figure out how to use the API.

During setup, we will need to walk people through:
Create testing account.
Create API key.
Give it to org or to repo.
Warning: If repo, will have to do it for every repo.

Conclusion

We'll probably end up implementing Option 2 at some point. At this point, I think the question is whether we use Option 1 temporarily so we can push out v3 and get people's tests working again.

@plocket
Copy link
Collaborator Author

plocket commented Dec 2, 2021

Decision

After some feedback, we're going to attempt to take a couple hours to implement option 1 for a quick fix and bump to v3, then work on option 2 later.

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 a pull request may close this issue.

1 participant