-
Notifications
You must be signed in to change notification settings - Fork 56
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
[ENG-5158] Add node addons acceptance tests #2264
[ENG-5158] Add node addons acceptance tests #2264
Conversation
826fbb3
to
51ffffc
Compare
@@ -197,30 +197,6 @@ module('Acceptance | guid-node/metadata', hooks => { | |||
await percySnapshot(assert); | |||
}); | |||
|
|||
test('Error handling: metadata', async function(this: TestContext, assert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what's going on with this test. It passes in the browser and command line, but fails when the tests are run in parallel. When run in parallel, it seems it can't find element #toast-container
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to skip
this and make a ticket to try to figure out what's going on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, looks very good. A couple of comments, but nothing to block merging unless we want to skip the deleted test.
// check connected accounts tab | ||
await click('[data-test-addons-tab-connected-accounts]'); | ||
await percySnapshot('Acceptance | guid-node/addons | Editing configured addons | connected accounts tab'); | ||
assert.dom('[data-test-addon-card]').exists({ count: 2 }, '2 providers with accounts are present'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just an aside, I vaguely recall that someone (either Mark, Abram, or both) suggested that we automatically switch to this tab on pageload if there are connected accounts. No need to change anything right now, but that may be coming in UAT.
// Skip until issue with added account not showing up is resolved | ||
skip('Adding new configured addons', async function(assert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't forget about this one when the time comes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or should we just try to fix it now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a ticket to fix this in a separate issue. I've made a note for myself in that ticket to un-skip this test in that ticket
@@ -197,30 +197,6 @@ module('Acceptance | guid-node/metadata', hooks => { | |||
await percySnapshot(assert); | |||
}); | |||
|
|||
test('Error handling: metadata', async function(this: TestContext, assert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be better to skip
this and make a ticket to try to figure out what's going on?
51ffffc
to
5dd5311
Compare
e7168fd
into
CenterForOpenScience:feature/addon-services
Purpose
Summary of Changes
Screenshot(s)
Side Effects
QA Notes