From: "Frédéric DALLEAU" <frederic.dalleau@linux.intel.com>
To: linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH 2/3] sco-tester: Introduce adapter features
Date: Mon, 09 Sep 2013 14:54:21 +0200 [thread overview]
Message-ID: <522DC4FD.1010601@linux.intel.com> (raw)
In-Reply-To: <20130909083356.GC29399@x220.p-661hnu-f1>
Hi Johan,
> First of all, you should use "false" here instead of "0" (and same for
> true vs 1 later).
>
> Secondly, whenever you want to manipulate some parameter of a test case
> it should not be done in the main function like that but instead you
> should have it as part of the test case data (struct sco_client_data)
> and create a separate instance for each test case.
>
> I do realize that the initial sco-tester didn't get this right (having
> disable_sco in struct test_data instead of struct sco_client_data), but
> it'd be good to get this fixed before adding any new test cases.
The problem is that the adapter feature has to be defined far before the
test is started.
Another problem is that sco_client_data is test dependant, eg. each
test can implement sco_client_data differently. In this case,
adapter_features could simply be undefined! This is already the case in
the first tests which do not use struct sco_client_data at all
(test_socket).
IMHO We do not want that. adapter_features must be defined explictly
before each test. This is mandatory.
In future I'm also thinking to implement some adapter "behavior". For
example : one test could be : try connect, emulator must accept, and
expected result is success. then try connect, emulator must refuse, and
result is expected to be connection refused.
My suggestion would be really different from yours :
I would write a big table with the elements of test_sco.
struct test {
char *name;
struct adapter_features *features;
struct adapter_behavior *behavior; /* If needed */
void (*test_cb)(struct test *test, void *data);
void *user_data; /* sco_client_data*/
int expected_result;
} tests [] = {...};
while (test[i].name) {
test_sco(test[i].name, test[i].test_cb, ...);
i++;
}
Let me know what you think.
Regards,
Frédéric
next prev parent reply other threads:[~2013-09-09 12:54 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-09 15:42 [PATCH 1/3] sco-tester: Update ECONNABORTED to EOPNOTSUPP Frédéric Dalleau
2013-08-09 15:42 ` [PATCH 2/3] sco-tester: Introduce adapter features Frédéric Dalleau
2013-09-09 8:33 ` Johan Hedberg
2013-09-09 12:54 ` Frédéric DALLEAU [this message]
2013-09-09 15:03 ` Johan Hedberg
2013-08-09 15:42 ` [PATCH 3/3] sco-tester: Test transparent data feature bit Frédéric Dalleau
2013-08-09 15:53 ` [PATCH 1/3] sco-tester: Update ECONNABORTED to EOPNOTSUPP Marcel Holtmann
2013-08-12 8:34 ` Frédéric DALLEAU
2013-08-12 23:22 ` Marcel Holtmann
2013-08-13 13:10 ` Frédéric DALLEAU
2013-09-02 15:53 ` Frédéric DALLEAU
2013-09-09 8:29 ` Johan Hedberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=522DC4FD.1010601@linux.intel.com \
--to=frederic.dalleau@linux.intel.com \
--cc=linux-bluetooth@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).