In the 'working' one, I see only two files that get pushed. In the other, I see more of the right ones. What the heck happened?

When this repo gets updated and new code gets added, some of the supporting files, like .gitignore, need to be changed. For example, when we add the ability to download files, we then need to ignore those in .gitignore.

Do we need another action to do that, or can we do that with the existing push action? We would at least have to handle a file not having changes so that won't trigger an error.

[See https://github.com/plocket/push-generated-file/issues/2]

It would be nice to be explicit about which params are optional.

Originally posted by @niharikasingh in #4 (comment)

The format is a comma separated list of words that are in the links or buttons that switch languages. Haven't figured out another way yet.

This might be specified in notes in the code, as indicated, but should also be in the README.

Originally issue posted by @niharikasingh in #51 (comment)

1. Don't run multiple tests in one Playground account at the same time, even if they're on different branches. That includes:
   1. Hitting the 'commit' button in the playground. That is, pushing a branch (pushing to an old branch or pushing to a new branch)
   2. Making a pull request
   3. Merging a pull request
2. Getting a 'timeout' error failure might mean you just have to run the test again. Look at what step the test failed on and what the screenshot from the error looks like.
3. If there is a spot the tests are often failing from a timeout in a particular spot, add a 'When I wait 1 second' or 'When I wait 2 seconds' just before the step that fails. There may be something that often takes some extra time there.

comment - this is not meant to detect breaking errors, just user-visible errors for user to fix

Originally posted by @niharikasingh in #33 (comment)

See plocket/push-generated-file#3

Good first issue if you know actions.

add back in?

Originally posted by @niharikasingh in #33 (comment)

If some link or button initiates downloading multiple separate files (I assume that's possible???) then the current code may not be able to account for that.

fix the return value - it should be an object of 2 strings

Originally posted by @niharikasingh in #33 (comment)

REPO_URL="${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}"

That or make it a secret. Hard to develop on a fork otherwise.

Apparently the field name attribute value is the base64 encoded version of the variable name. That can be useful for people handling translations through separate .yml files as it can be language agnostic.

Not urgent, but I wonder if we should be using a global variable for the permitted timeout, and then this should just be env_vars.TIMEOUT - 2000 or something.

Originally posted by @niharikasingh in #23 (comment)

The eventual goal would be a step to compare an existing pdf to a downloaded pdf. First we have to figure out how to compare two PDFs, though.

Do we do that already? Something keeps timing out in MADE between "rental agreement" and "tenancy facts" and it fails pretty quickly even though continuing waits for navigation.

some clarifying comments here would be helpful to understand the output at each stage

Originally posted by @niharikasingh in #36 (comment)

Whether checkbox is checked or not, etc. Should be able to base this off of current var steps.

"I log {string}" or something

Add a random suffix to the project name. Rationale: If you make two pushes really close together, it shouldn't be a reason for your tests to fail.

comment out/remove? since this is not used anymore.

Originally posted by @niharikasingh in #36 (comment)

We may need it later, but it'll be easy to find then and right now it's premature.

This is an alternative to having the developer write out every scenario to cover all their code. It's not ideal, but since you can't abstract in cucumber, writing every single scenario can be a huge task. It is possible, in cucumber, to allow the user to pass in data structures like lists. We would just have to handle randomly selecting them.

Note: This is not a fault in cucumber - it's not meant to be used the way we're using it.

Also need to think whether the developer will need to copy/paste this 'scenario' for however many times they want the random tests to be run, or if we can run them repeatedly somehow. This might be better in its own issue. [Edit: This is probably doable now that we have the knowledge of setting, and resetting, our own custom timeouts.]

• Add workflow to make PR with latest package version?

This doesn't seem to list the methodology you ended up using here. I think it would be awesome to have a summary of how this function works, because it is so complicated.

Originally posted by @niharikasingh in #4 (comment)

Right now, takedown leaves the language files untouched

https://github.com/plocket/docassemble-cucumber/blob/0faa730c8fbe28b990c6147b29688eb5844441a0/lib/steps.js#L330

We can remove //text() from there in combination with removing /.. to get the button node.

Right now, setting up multilingual tests would be a real pain. You'd have to repeat each test for each different language, even if you were using variable names to test - all the steps would be the same except the first step where you'd specify a language. That's a lot of tests and a lot to maintain.

What are ideas for how to make it easier? We've come up with the one that we currently think will be easiest for developers using this stuff, but are open to other thoughts. This package here would implement this functionality in the dependent repo.

Prep

1. Allow the developer to specify their desired languages (probably as an optional environment variable)
2. .gitignore a specific folder in the features folder. Currently making the files right in the features folder, so this isn't needed. May be worth discussing.

Additional npm script

First, make the ignored folder in the features folder. Then, for each language listed, do this:

1. In the ignored folder, add a folder with the code of the language.
2. Duplicate all testing files that are not in the ignored folder.
3. Copy them into this language folder.
4. Add a step to each scenario that would tap the button of the correct language on the intro page.

Add this npm script to the testing action just before running the tests. Maybe add another script to the package.json that would include the build step when it runs the tests.

Stretch goals

1. Allow the developer to specify a language or list of languages when manually running the tests (once manual test running is available).

Questions

1. If the default language isn't specified on the list, should we find a way to only run the specified languages?
2. There was something else that seemed significant, but I don't remember what it is.

This might be a personal preference, but I think this line is confusing. I think this might be one of the rare cases where a for (var i = 0; i < roles.length; i++) loop is clearer, because then this comparison becomes much simpler.

Originally posted by @niharikasingh in #4 (comment)

Add explicit error handling for names with too many parts.

Originally posted by @niharikasingh in #36 (comment)

Same for addresses that don't have the correct format.

Testing someone else's version/project can also be useful. In that case, setup should not make a new project and takedown should not try to delete it.

pointer to docassemble code would be nice, so people know the B/R distinction exists.

Originally posted by @niharikasingh in #33 (comment)

For local developers. Might as well make it available.

change string to something like "will never match existing dom node"

Originally posted by @niharikasingh in #33 (comment)

Developers should write tests so that they actually set all the unimportant default field values in each test. When interviews get translated, they often lose those defaults because translators edit them. If the default isn't particularly important, it shouldn't cause the test to fail.

The only reason to leave a default is if the default is crucial and its absence should cause an error.

In that case, though, they should write a test that checks that the field's default state is as it should be when the page loads instead of relying on side-effect errors bringing that up. Those steps aren't defined yet, but we should make that into its own issue.

Situation:

1. Developer makes a default for a checkbox that makes it 'checked' when the user first comes to the page.
2. Translator accidentally translate the words that set the default, leaving the checkbox without a default, meaning it starts as 'unchecked'. Let's call the language 'Foonian'
3. Test comes along. To test default language behavior, it does not tap the checkbox, leaving it checked.
4. Automated language tests come along and build the 'Foonian' test. That test also leaves the checkbox alone, which means the checkbox is unchecked.
5. The checkbox determines other behavior in the interview and so the tests in 'Foonian' appear to be broken, even though that default isn't crucial to the functioning of the interview.

It's going to be a bit of a pain to add that. I can't think of another way. It may mean that we really do only need one tests for setting any variable, so that might be a plus...

We're still having trouble getting an interview to start for the first test in the series. The interview file doesn't load for the first test even though the tests after the first one do start. That sounds like the server may have started for 'test.yml', but it hasn't finished loading everything. We can do something similar, though less extreme, to what we do with 'test.yml' to give it a couple extra shots.

Let's call it 3 or so times with 20 or 30 seconds between each.

[It would fail because the server was not working (since test.yml exists in the Project's files already and isn't dependent on the GitHub code).] This would be after we implement a more robust way of loading the interview initially. Track #52 for this.

Maybe look at https://github.com/cucumber/cucumber-js/blob/master/docs/support_files/hooks.md Also maybe CLI options, though I haven't found docs on those yet.

[CLI options: https://github.com/cucumber/cucumber-js/blob/master/docs/cli.md]

[We actually probably want to do this in the actions script. Really want to figure out how we can set up that script in a way that lets us change it when we need to instead of people having to update their action scripts. Bleh.]

That's because the input's name will be based on the language of the question. We may implement 'first', 'second', etc. later, but it's not available right now and it's not a good coding practice.

I overloaded the 'Given' step a bit. We can possibly break out:

1. Choosing a device: "I start on mobile" or something.
2. Possibly break out choosing a language into its own step too, though it might complicate the automated language selection step, so may not be worth it.

Thoughts?

this should always exist because of the assertion in line 127

Originally posted by @niharikasingh in #33 (comment)

We might be able to speed it up with XPath or something.

We might look into whether <meta itemprop="name" content="docassemble: Error"> is consistently present on actually broken pages. Would really speed up error finding. This might at least speed up finding one type of error.

This can probably just compare text. Not sure about signature.