Skip to content

GPII-1839 Using the Dynamic Device Reporter when running acceptance tests#461

Closed
klown wants to merge 13 commits intoGPII:masterfrom
klown:GPII-1839
Closed

GPII-1839 Using the Dynamic Device Reporter when running acceptance tests#461
klown wants to merge 13 commits intoGPII:masterfrom
klown:GPII-1839

Conversation

@klown
Copy link
Copy Markdown
Member

@klown klown commented Jun 24, 2016

@kaspermarkus One of the work items discussed at the June face-to-face was to use the dynamic device reporter in acceptance testing, and not the mock version. I volunteered to make the changes. I've done two things here:

  • changed some basic testing config files to specify the device-reporter-in-production-mode as opposed to development mode.
  • added "deviceReporter" blocks to a number of testSpec files so that the device reporter is run in those tests.

I'm not 100% sure I've found everything that needs changing, but this is ready for review. Let me know if more changes are needed.

klown added 4 commits June 17, 2016 15:52
Modified "basic" configuration files such that the device reporter is
configured/loaded in production mode as opposed to development mode.
Added a "deviceReporter" block to test definitions for the built-in,
and orca test specs.
Added a "deviceReporter" block to test definitions for the built-in,
jaws, maavis, and nvda test specs.
@gpii-bot
Copy link
Copy Markdown

Could one of the admins verify that these changes are reasonable to test? If so, please reply with "ok to test".

@colinbdclark
Copy link
Copy Markdown
Member

ok to test

@gpii-bot
Copy link
Copy Markdown

CI job passed.

@kaspermarkus kaspermarkus self-assigned this Sep 21, 2016
Copy link
Copy Markdown
Member

@kaspermarkus kaspermarkus left a comment

Choose a reason for hiding this comment

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

I tried running the NVDA acceptance tests in windows - but on a machine without NVDA installed - and they exploded (since it couldn't find the config file)... I think a good chunk of the purpose of this JIRA is to avoid that the tests explodes if you dont have ALL the 3rd party ATs installed on your device when running acceptance tests. So in my case for example, the acceptance test should pass - but probably print out to the console that NVDA was not installed and that was the reason for passing

}
},
"mergeConfigs": "%universal/gpii/configs/gpii.config.development.all.local.json"
"mergeConfigs": "%universal/gpii/configs/gpii.config.all.development.dr.production.json"
Copy link
Copy Markdown
Member

@kaspermarkus kaspermarkus Sep 21, 2016

Choose a reason for hiding this comment

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

It seems that this file is now identical to "gpii.tests.acceptance.dynamicDeviceReporter.config.json" - so it seems sensible to remove one of them.. perhaps this one and then use the dynamicDeviceReporter instead and update references and docs accordingly

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@kaspermarkus I need clarification on what "this one" is. Which two config files are identical? Which one are you suggesting should be deleted? I can find no file with the name you gave above:"gpii.tests.acceptance.dynamicDeviceReporter.config.json".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@kaspermarkus I figured it out. You meant that "gpii.tests.acceptance.localInstall.dynamicDeviceReporter.config.json" is identical to "gpii.tests.acceptance.localInstall.confg.json". I'll delete the latter and replace it with the former, and update docs etc.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

"%flatMatchMaker/configs/gpii.flatMatchMaker.config.development.json",
"%rawPreferencesServer/configs/gpii.rawPreferencesServer.config.development.json",
"%deviceReporter/configs/gpii.deviceReporter.config.development.json"
"%deviceReporter/configs/gpii.deviceReporter.config.production.json"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this a change that you have made? And if so, what's the reason?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, this is a change I made. The reason being that this JIRA is about using the production version of the device reporter for acceptance testing. I'm guessing from your comment that the proposed change is not specific to testing, or has nothing to do with testing at all. Is that correct?

@gpii-bot
Copy link
Copy Markdown

CI job failed. Please visit http://lists.gpii.net/pipermail/ci/ for more details.

@klown
Copy link
Copy Markdown
Member Author

klown commented Sep 29, 2016

@kaspermarkus Using your fix for GPII-2006 helps, but the tests are still exploding in at least two other places. First, the setting handlers still attempt to set the contents of the INI file to the test values, but an exception is thrown since the file doesn't exist, and the test fails since the actual payload is a set of settings all of which are "undefined". Later, there is an attempt to kill the NVDA process, which isn't running, and that explodes/fails as well.

I'm working on a fix, and looking for similar gotchas.

Don't explode on writing to non-existent settings file.
@gpii-bot
Copy link
Copy Markdown

CI job failed. Please visit http://lists.gpii.net/pipermail/ci/ for more details.

gpii.settingsHandlers.writeFile = function (content, options) {
if (!options || !options.filename) {
fluid.fail("writeFile: expected an options block defining filename and encoding");
} else if (!fs.existsSync(options.filename)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems odd - why wouldn't we want to support writing a file which didn't previously exist? I imagine this branch would break the work that @javihernandez would like to do with #462

Copy link
Copy Markdown
Member Author

@klown klown Nov 1, 2016

Choose a reason for hiding this comment

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

There are two cases of non-existence. The case here is when the solution is not installed and there is no settings file to write. This is the flip side of using the solution to GPII-2006, where the reading of the file fails for the same reason. @kaspermarkus wrote above:

I tried running the NVDA acceptance tests in windows - but on a machine without NVDA installed - and they exploded (since it couldn't find the config file)...

See also my comment from Sep 24:

... the setting handlers still attempt to set the contents of the INI file to the test values, but an exception is thrown since the file doesn't exist ...

That happens because the settings file is within a directory structure that doesn't exist if the solution is not installed, something like "...\nvda\nvda.ini" There is no parent "nvda" directory. I'll update the unit test for writing to a non-existent file to reflect that.

The other sense of non-existence is where the solution is installed, but needs to be run once to create the settings file. In that case, writing (creating) the settings file should be supported.

We need to handle both cases. The system should be sensitive to the context -- whether the solution is installed -- and react appropriately.

Added some directory structure to better reflect the situation
when a settings file does not exist because the solution is not
installed.
@gpii-bot
Copy link
Copy Markdown

gpii-bot commented Nov 1, 2016

CI job failed. Please visit http://lists.gpii.net/pipermail/ci/ for more details.

- replaced base gpii.tests.acceptance.localInstall.config.json|txt with gpii.tests.acceptance.localInstall.dynamicDeviceReporter.config.json|txt
- updated dependant configuration files to reference the new dynamicDeviceReporter base configuration file.
- updated all the associated ".txt" documentation files.
@gpii-bot
Copy link
Copy Markdown

gpii-bot commented Nov 9, 2016

CI job failed. Please visit http://lists.gpii.net/pipermail/ci/ for more details.

@gpii-bot
Copy link
Copy Markdown

CI job failed. Please visit http://lists.gpii.net/pipermail/ci/ for more details.

@gtirloni
Copy link
Copy Markdown
Contributor

gtirloni commented Feb 3, 2017

ok to test

@gpii-bot
Copy link
Copy Markdown

gpii-bot commented Feb 3, 2017

CI job failed. Please visit http://lists.gpii.net/pipermail/ci/ for more details.

@javihernandez
Copy link
Copy Markdown
Member

New pull request is #494

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.

7 participants