linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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


  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).